summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2021-10-01 21:11:37 +0000
committerGitLab Bot <gitlab-bot@gitlab.com>2021-10-01 21:11:37 +0000
commitfd7da8784c9b0f79dc5afeb83bf2313c2cdc1ffc (patch)
tree5726a2f9649d04b273b5d4e00421c87e0375216c
parent0f295cd16f516ec10e6cd0b3fa5846563c08d9b8 (diff)
downloadgitlab-ce-fd7da8784c9b0f79dc5afeb83bf2313c2cdc1ffc.tar.gz
Add latest changes from gitlab-org/gitlab@master
Diffstat
-rw-r--r--app/assets/javascripts/invite_members/components/invite_members_modal.vue13
-rw-r--r--app/assets/javascripts/invite_members/utils/response_message_parser.js5
-rw-r--r--app/models/concerns/restricted_signup.rb42
-rw-r--r--app/models/member.rb8
-rw-r--r--app/models/user.rb4
-rw-r--r--app/services/projects/transfer_service.rb22
-rw-r--r--config/feature_flags/development/specialized_worker_for_project_transfer_auth_recalculation.yml8
-rw-r--r--config/locales/en.yml3
-rw-r--r--doc/operations/feature_flags.md4
-rw-r--r--doc/operations/index.md4
-rw-r--r--doc/operations/metrics/alerts.md6
-rw-r--r--doc/operations/metrics/embed.md4
-rw-r--r--doc/operations/metrics/index.md2
-rw-r--r--doc/operations/tracing.md4
-rw-r--r--locale/gitlab.pot37
-rw-r--r--spec/frontend/invite_members/components/invite_members_modal_spec.js4
-rw-r--r--spec/frontend/invite_members/mock_data/api_responses.js14
-rw-r--r--spec/frontend/invite_members/utils/response_message_parser_spec.js28
-rw-r--r--spec/models/member_spec.rb8
-rw-r--r--spec/models/user_spec.rb14
-rw-r--r--spec/services/members/create_service_spec.rb2
-rw-r--r--spec/services/members/invite_service_spec.rb2
-rw-r--r--spec/services/projects/transfer_service_spec.rb64
-rw-r--r--workhorse/internal/upstream/upstream.go18
-rw-r--r--workhorse/internal/upstream/upstream_test.go99
25 files changed, 265 insertions, 154 deletions
diff --git a/app/assets/javascripts/invite_members/components/invite_members_modal.vue b/app/assets/javascripts/invite_members/components/invite_members_modal.vue
index ab42e8cdfeb..cd0b413265b 100644
--- a/app/assets/javascripts/invite_members/components/invite_members_modal.vue
+++ b/app/assets/javascripts/invite_members/components/invite_members_modal.vue
@@ -11,9 +11,10 @@ import {
GlFormInput,
GlFormCheckboxGroup,
} from '@gitlab/ui';
-import { partition, isString } from 'lodash';
+import { partition, isString, unescape } from 'lodash';
import Api from '~/api';
import ExperimentTracking from '~/experimentation/experiment_tracking';
+import { sanitize } from '~/lib/dompurify';
import { BV_SHOW_MODAL } from '~/lib/utils/constants';
import { s__, sprintf } from '~/locale';
import {
@@ -293,7 +294,7 @@ export default {
};
},
conditionallyShowToastSuccess(response) {
- const message = responseMessageFromSuccess(response);
+ const message = this.unescapeMsg(responseMessageFromSuccess(response));
if (message === '') {
this.showToastMessageSuccess();
@@ -309,13 +310,17 @@ export default {
this.closeModal();
},
showInvalidFeedbackMessage(response) {
+ const message = this.unescapeMsg(responseMessageFromError(response));
+
this.isLoading = false;
- this.invalidFeedbackMessage =
- responseMessageFromError(response) || this.$options.labels.invalidFeedbackMessageDefault;
+ this.invalidFeedbackMessage = message || this.$options.labels.invalidFeedbackMessageDefault;
},
handleMembersTokenSelectClear() {
this.invalidFeedbackMessage = '';
},
+ unescapeMsg(message) {
+ return unescape(sanitize(message, { ALLOWED_TAGS: [] }));
+ },
},
labels: {
members: {
diff --git a/app/assets/javascripts/invite_members/utils/response_message_parser.js b/app/assets/javascripts/invite_members/utils/response_message_parser.js
index b7bc9ea5652..52ec3be3205 100644
--- a/app/assets/javascripts/invite_members/utils/response_message_parser.js
+++ b/app/assets/javascripts/invite_members/utils/response_message_parser.js
@@ -18,7 +18,10 @@ function responseMessageStringForMultiple(message) {
return message.includes(':');
}
function responseMessageStringFirstPart(message) {
- return message.split(' and ')[0];
+ const firstPart = message.split(':')[1];
+ const firstMsg = firstPart.split(/ and [\w-]*$/)[0].trim();
+
+ return firstMsg;
}
export function responseMessageFromError(response) {
diff --git a/app/models/concerns/restricted_signup.rb b/app/models/concerns/restricted_signup.rb
index 587f8c35ff7..cf97be21165 100644
--- a/app/models/concerns/restricted_signup.rb
+++ b/app/models/concerns/restricted_signup.rb
@@ -7,15 +7,49 @@ module RestrictedSignup
def validate_admin_signup_restrictions(email)
return if allowed_domain?(email)
+ error_type = fetch_error_type(email)
+
+ return unless error_type.present?
+
+ [
+ signup_email_invalid_message,
+ error_message[created_by_key][error_type]
+ ].join(' ')
+ end
+
+ def fetch_error_type(email)
if allowlist_present?
- return _('domain is not authorized for sign-up.')
+ :allowlist
elsif denied_domain?(email)
- return _('is not from an allowed domain.')
+ :denylist
elsif restricted_email?(email)
- return _('is not allowed. Try again with a different email address, or contact your GitLab admin.')
+ :restricted
end
+ end
+
+ def error_message
+ {
+ admin: {
+ allowlist: html_escape_once(_("Go to the 'Admin area &gt; Sign-up restrictions', and check 'Allowed domains for sign-ups'.")).html_safe,
+ denylist: html_escape_once(_("Go to the 'Admin area &gt; Sign-up restrictions', and check the 'Domain denylist'.")).html_safe,
+ restricted: html_escape_once(_("Go to the 'Admin area &gt; Sign-up restrictions', and check 'Email restrictions for sign-ups'.")).html_safe,
+ group_setting: html_escape_once(_("Go to the group’s 'Settings &gt; General' page, and check 'Restrict membership by email domain'.")).html_safe
+ },
+ nonadmin: {
+ allowlist: error_nonadmin,
+ denylist: error_nonadmin,
+ restricted: error_nonadmin,
+ group_setting: error_nonadmin
+ }
+ }
+ end
+
+ def error_nonadmin
+ _("Check with your administrator.")
+ end
- nil
+ def created_by_key
+ created_by&.can_admin_all_resources? ? :admin : :nonadmin
end
def denied_domain?(email)
diff --git a/app/models/member.rb b/app/models/member.rb
index c1b6a6b11fe..45af34a3e6b 100644
--- a/app/models/member.rb
+++ b/app/models/member.rb
@@ -448,6 +448,14 @@ class Member < ApplicationRecord
errors.add(:user, error) if error
end
+ def signup_email_invalid_message
+ if source_type == 'Project'
+ _("is not allowed for this project.")
+ else
+ _("is not allowed for this group.")
+ end
+ end
+
def update_highest_role?
return unless user_id.present?
diff --git a/app/models/user.rb b/app/models/user.rb
index 832f8c879af..f56506066ae 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -2102,6 +2102,10 @@ class User < ApplicationRecord
errors.add(:email, error) if error
end
+ def signup_email_invalid_message
+ _('is not allowed for sign-up.')
+ end
+
def check_username_format
return if username.blank? || Mime::EXTENSION_LOOKUP.keys.none? { |type| username.end_with?(".#{type}") }
diff --git a/app/services/projects/transfer_service.rb b/app/services/projects/transfer_service.rb
index 4764462e06f..a2028cf8a19 100644
--- a/app/services/projects/transfer_service.rb
+++ b/app/services/projects/transfer_service.rb
@@ -156,19 +156,15 @@ module Projects
user_ids = @old_namespace.user_ids_for_project_authorizations |
@new_namespace.user_ids_for_project_authorizations
- if Feature.enabled?(:specialized_worker_for_project_transfer_auth_recalculation)
- AuthorizedProjectUpdate::ProjectRecalculateWorker.perform_async(project.id)
-
- # Until we compare the inconsistency rates of the new specialized worker and
- # the old approach, we still run AuthorizedProjectsWorker
- # but with some delay and lower urgency as a safety net.
- UserProjectAccessChangedService.new(user_ids).execute(
- blocking: false,
- priority: UserProjectAccessChangedService::LOW_PRIORITY
- )
- else
- UserProjectAccessChangedService.new(user_ids).execute
- end
+ AuthorizedProjectUpdate::ProjectRecalculateWorker.perform_async(project.id)
+
+ # Until we compare the inconsistency rates of the new specialized worker and
+ # the old approach, we still run AuthorizedProjectsWorker
+ # but with some delay and lower urgency as a safety net.
+ UserProjectAccessChangedService.new(user_ids).execute(
+ blocking: false,
+ priority: UserProjectAccessChangedService::LOW_PRIORITY
+ )
end
def rollback_side_effects
diff --git a/config/feature_flags/development/specialized_worker_for_project_transfer_auth_recalculation.yml b/config/feature_flags/development/specialized_worker_for_project_transfer_auth_recalculation.yml
deleted file mode 100644
index b77ed607501..00000000000
--- a/config/feature_flags/development/specialized_worker_for_project_transfer_auth_recalculation.yml
+++ /dev/null
@@ -1,8 +0,0 @@
----
-name: specialized_worker_for_project_transfer_auth_recalculation
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/61967
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/334237
-milestone: '14.1'
-type: development
-group: group::access
-default_enabled: false
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 4615c9a7390..233dca33bb8 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -10,6 +10,9 @@ en:
target: Target issue
group:
path: Group URL
+ member:
+ user: "The member's email address"
+ invite_email: "The member's email address"
project/error_tracking_setting:
token: "Auth Token"
project: "Project"
diff --git a/doc/operations/feature_flags.md b/doc/operations/feature_flags.md
index a8686e2f4b7..79a2e984deb 100644
--- a/doc/operations/feature_flags.md
+++ b/doc/operations/feature_flags.md
@@ -7,7 +7,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w
# Feature Flags **(FREE)**
> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/7433) in GitLab 11.4.
-> - [Moved](https://gitlab.com/gitlab-org/gitlab/-/issues/212318) to GitLab Free in 13.5.
+> - [Moved](https://gitlab.com/gitlab-org/gitlab/-/issues/212318) from GitLab Premium to GitLab Free in 13.5.
With Feature Flags, you can deploy your application's new features to production in smaller batches.
You can toggle a feature on and off to subsets of users, helping you achieve Continuous Delivery.
@@ -312,7 +312,7 @@ Unleash currently [offers many SDKs for various languages and frameworks](https:
For API content, see:
- [Feature Flags API](../api/feature_flags.md)
-- [Feature Flag Specs API](../api/feature_flag_specs.md) (Deprecated and [scheduled for removal in GitLab 14.0](https://gitlab.com/gitlab-org/gitlab/-/issues/213369).)
+- [Feature Flag Specs API](../api/feature_flag_specs.md) (Deprecated and [scheduled for removal](https://gitlab.com/gitlab-org/gitlab/-/issues/213369) in GitLab 14.0.)
- [Feature Flag User Lists API](../api/feature_flag_user_lists.md)
- [Legacy Feature Flags API](../api/feature_flags_legacy.md)
diff --git a/doc/operations/index.md b/doc/operations/index.md
index d170e82dd7f..44737600c03 100644
--- a/doc/operations/index.md
+++ b/doc/operations/index.md
@@ -36,7 +36,7 @@ Are your alerts too noisy? Alerts configured on GitLab metrics can configured
and fine-tuned in GitLab immediately following a fire-fight.
- [Manage alerts and incidents](incident_management/index.md) in GitLab.
-- [Configure alerts for metrics](metrics/alerts.md#set-up-alerts-for-prometheus-metrics) in GitLab.
+- [Configure alerts for metrics](metrics/alerts.md) in GitLab.
- Create a [status page](incident_management/status_page.md)
to communicate efficiently to your users during an incident.
@@ -96,4 +96,4 @@ an environment.
- Deploy to different [environments](../ci/environments/index.md).
- Connect your project to a [Kubernetes cluster](../user/project/clusters/index.md).
- See how your application is used and analyze events with [Product Analytics](product_analytics.md).
-- Create, toggle, and remove [Feature Flags](feature_flags.md). **(PREMIUM)**
+- Create, toggle, and remove [Feature Flags](feature_flags.md).
diff --git a/doc/operations/metrics/alerts.md b/doc/operations/metrics/alerts.md
index a3d741b78ea..44999ad7cd6 100644
--- a/doc/operations/metrics/alerts.md
+++ b/doc/operations/metrics/alerts.md
@@ -61,7 +61,7 @@ Prometheus server to use the
## Trigger actions from alerts **(ULTIMATE)**
-> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/4925) in [GitLab Ultimate](https://about.gitlab.com/pricing/) 11.11.
+> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/4925) in GitLab 11.11.
Alerts can be used to trigger actions, like opening an issue automatically
(disabled by default since `13.1`). To configure the actions:
@@ -83,7 +83,7 @@ values extracted from the [`alerts` field in webhook payload](https://prometheus
- `full_query`: Alert query extracted from the payload's `generatorURL` field
- Optional list of attached annotations extracted from `annotations/*`
- Alert [GFM](../../user/markdown.md): GitLab Flavored Markdown from the payload's `annotations/gitlab_incident_markdown` field.
-- Alert Severity (introduced in GitLab version [13.9](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/50871):
+- Alert Severity ([Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/50871) in GitLab version 13.9):
Extracted from the alert payload field `labels/severity`. Maps case-insensitive
value to [Alert's severity](../incident_management/alerts.md#alert-severity):
- **Critical**: `critical`, `s1`, `p1`, `emergency`, `fatal`, or any value not in this list
@@ -107,7 +107,7 @@ of the project.
### Recovery alerts
-> - [From GitLab Ultimate 12.5](https://gitlab.com/gitlab-org/gitlab/-/issues/13401), when GitLab receives a recovery alert, it automatically closes the associated issue.
+> [From GitLab 12.5](https://gitlab.com/gitlab-org/gitlab/-/issues/13401), when GitLab receives a recovery alert, it automatically closes the associated issue.
The alert in GitLab will be automatically resolved when Prometheus
sends a payload with the field `status` set to `resolved`.
diff --git a/doc/operations/metrics/embed.md b/doc/operations/metrics/embed.md
index d773d04f1cc..e40e8864319 100644
--- a/doc/operations/metrics/embed.md
+++ b/doc/operations/metrics/embed.md
@@ -96,8 +96,8 @@ a chart corresponding to the query can be included if these requirements are met
## Embedding cluster health charts
-> - [Introduced](<https://gitlab.com/gitlab-org/gitlab/-/issues/40997>) in [GitLab Ultimate](https://about.gitlab.com/pricing/) 12.9.
-> - [Moved](<https://gitlab.com/gitlab-org/gitlab/-/issues/208224>) to GitLab core in 13.2.
+> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/40997) in GitLab 12.9.
+> - [Moved](https://gitlab.com/gitlab-org/gitlab/-/issues/208224) from GitLab Ultimate to GitLab Free in 13.2.
[Cluster Health Metrics](../../user/project/clusters/index.md#visualizing-cluster-health)
can also be embedded in [GitLab-flavored Markdown](../../user/markdown.md).
diff --git a/doc/operations/metrics/index.md b/doc/operations/metrics/index.md
index 07e71fe79ef..f09b9f35d88 100644
--- a/doc/operations/metrics/index.md
+++ b/doc/operations/metrics/index.md
@@ -111,7 +111,7 @@ dashboard is visible to authenticated and non-authenticated users.
## Adding custom metrics
-> - [Moved](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/28527) to GitLab Free in 12.10.
+> - [Moved](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/28527) from GitLab Premium to GitLab Free in 12.10.
Custom metrics can be monitored by adding them on the monitoring dashboard page.
After saving them, they display on the environment metrics dashboard provided that either:
diff --git a/doc/operations/tracing.md b/doc/operations/tracing.md
index 7435c0dd8a2..1593607cc98 100644
--- a/doc/operations/tracing.md
+++ b/doc/operations/tracing.md
@@ -6,8 +6,8 @@ info: To determine the technical writer assigned to the Stage/Group associated w
# Tracing **(FREE)**
-> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/7903) in GitLab Ultimate 11.5.
-> - [Moved](https://gitlab.com/gitlab-org/gitlab/-/issues/42645) to GitLab Free in 13.5.
+> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/7903) in GitLab 11.5.
+> - [Moved](https://gitlab.com/gitlab-org/gitlab/-/issues/42645) from GitLab Ultimate to GitLab Free in 13.5.
Tracing provides insight into the performance and health of a deployed application, tracking each
function or microservice that handles a given request. Tracing makes it easy to understand the
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index d58c676d686..ce2a62d0cf9 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -6546,6 +6546,9 @@ msgstr ""
msgid "Check the elasticsearch.log file to debug why the migration was halted and make any changes before retrying the migration. When you fix the cause of the failure, click \"Retry migration\", and the migration will be scheduled to be retried in the background."
msgstr ""
+msgid "Check with your administrator."
+msgstr ""
+
msgid "Check your Docker images for known vulnerabilities."
msgstr ""
@@ -7204,9 +7207,6 @@ msgstr ""
msgid "Cloud Run description and apps that are suitable for this deployment target"
msgstr ""
-msgid "Cloud licenses can not be removed."
-msgstr ""
-
msgid "Cluster"
msgstr ""
@@ -15823,9 +15823,21 @@ msgstr ""
msgid "Go to snippets"
msgstr ""
+msgid "Go to the 'Admin area &gt; Sign-up restrictions', and check 'Allowed domains for sign-ups'."
+msgstr ""
+
+msgid "Go to the 'Admin area &gt; Sign-up restrictions', and check 'Email restrictions for sign-ups'."
+msgstr ""
+
+msgid "Go to the 'Admin area &gt; Sign-up restrictions', and check the 'Domain denylist'."
+msgstr ""
+
msgid "Go to the activity feed"
msgstr ""
+msgid "Go to the group’s 'Settings &gt; General' page, and check 'Restrict membership by email domain'."
+msgstr ""
+
msgid "Go to the milestone list"
msgstr ""
@@ -40028,9 +40040,6 @@ msgstr ""
msgid "does not have a supported extension. Only %{extension_list} are supported"
msgstr ""
-msgid "domain is not authorized for sign-up."
-msgstr ""
-
msgid "download it"
msgstr ""
@@ -40048,11 +40057,6 @@ msgstr ""
msgid "email '%{email}' is not a verified email."
msgstr ""
-msgid "email does not match the allowed domain of %{email_domains}"
-msgid_plural "email does not match the allowed domains: %{email_domains}"
-msgstr[0] ""
-msgstr[1] ""
-
msgid "enabled"
msgstr ""
@@ -40266,16 +40270,19 @@ msgstr ""
msgid "is not a valid X509 certificate."
msgstr ""
-msgid "is not allowed since the group is not top-level group."
+msgid "is not allowed for sign-up."
msgstr ""
-msgid "is not allowed. Try again with a different email address, or contact your GitLab admin."
+msgid "is not allowed for this group."
msgstr ""
-msgid "is not allowed. We do not currently support project-level iterations"
+msgid "is not allowed for this project."
msgstr ""
-msgid "is not from an allowed domain."
+msgid "is not allowed since the group is not top-level group."
+msgstr ""
+
+msgid "is not allowed. We do not currently support project-level iterations"
msgstr ""
msgid "is not in the group enforcing Group Managed Account"
diff --git a/spec/frontend/invite_members/components/invite_members_modal_spec.js b/spec/frontend/invite_members/components/invite_members_modal_spec.js
index 95b1c55b82d..8c3c549a5eb 100644
--- a/spec/frontend/invite_members/components/invite_members_modal_spec.js
+++ b/spec/frontend/invite_members/components/invite_members_modal_spec.js
@@ -242,7 +242,7 @@ describe('InviteMembersModal', () => {
};
const expectedEmailRestrictedError =
- "email 'email@example.com' does not match the allowed domains: example1.org";
+ "The member's email address is not allowed for this project. Go to the Admin area > Sign-up restrictions, and check Allowed domains for sign-ups.";
const expectedSyntaxError = 'email contains an invalid email address';
it('calls the API with the expected focus data when an areas_of_focus checkbox is clicked', () => {
@@ -421,7 +421,7 @@ describe('InviteMembersModal', () => {
await waitForPromises();
expect(membersFormGroupInvalidFeedback()).toBe(
- "root: User email 'admin@example.com' does not match the allowed domain of example2.com",
+ "The member's email address is not allowed for this project. Go to the Admin area > Sign-up restrictions, and check Allowed domains for sign-ups.",
);
expect(findMembersSelect().props('validationState')).toBe(false);
});
diff --git a/spec/frontend/invite_members/mock_data/api_responses.js b/spec/frontend/invite_members/mock_data/api_responses.js
index 79b56a33708..dd84b4fd78f 100644
--- a/spec/frontend/invite_members/mock_data/api_responses.js
+++ b/spec/frontend/invite_members/mock_data/api_responses.js
@@ -9,7 +9,7 @@ const INVITATIONS_API_ERROR_EMAIL_INVALID = {
const INVITATIONS_API_EMAIL_RESTRICTED = {
message: {
'email@example.com':
- "Invite email 'email@example.com' does not match the allowed domains: example1.org",
+ "The member's email address is not allowed for this project. Go to the Admin area > Sign-up restrictions, and check Allowed domains for sign-ups.",
},
status: 'error',
};
@@ -17,9 +17,9 @@ const INVITATIONS_API_EMAIL_RESTRICTED = {
const INVITATIONS_API_MULTIPLE_EMAIL_RESTRICTED = {
message: {
'email@example.com':
- "Invite email email 'email@example.com' does not match the allowed domains: example1.org",
+ "The member's email address is not allowed for this project. Go to the Admin area > Sign-up restrictions, and check Allowed domains for sign-ups.",
'email4@example.com':
- "Invite email email 'email4@example.com' does not match the allowed domains: example1.org",
+ "The member's email address is not allowed for this project. Go to the Admin area > Sign-up restrictions, and check the Domain denylist.",
},
status: 'error',
};
@@ -36,7 +36,11 @@ const MEMBERS_API_MEMBER_ALREADY_EXISTS = {
};
const MEMBERS_API_SINGLE_USER_RESTRICTED = {
- message: { user: ["email 'email@example.com' does not match the allowed domains: example1.org"] },
+ message: {
+ user: [
+ "The member's email address is not allowed for this project. Go to the Admin area > Sign-up restrictions, and check Allowed domains for sign-ups.",
+ ],
+ },
};
const MEMBERS_API_SINGLE_USER_ACCESS_LEVEL = {
@@ -49,7 +53,7 @@ const MEMBERS_API_SINGLE_USER_ACCESS_LEVEL = {
const MEMBERS_API_MULTIPLE_USERS_RESTRICTED = {
message:
- "root: User email 'admin@example.com' does not match the allowed domain of example2.com and user18: User email 'user18@example.org' does not match the allowed domain of example2.com",
+ "root: The member's email address is not allowed for this project. Go to the Admin area > Sign-up restrictions, and check Allowed domains for sign-ups. and user18: The member's email address is not allowed for this project. Go to the Admin area > Sign-up restrictions, and check the Domain denylist. and john_doe31: The member's email address is not allowed for this project. Go to the Admin area > Sign-up restrictions, and check Email restrictions for sign-ups.",
status: 'error',
};
diff --git a/spec/frontend/invite_members/utils/response_message_parser_spec.js b/spec/frontend/invite_members/utils/response_message_parser_spec.js
index 3c88b5a2418..e2cc87c8547 100644
--- a/spec/frontend/invite_members/utils/response_message_parser_spec.js
+++ b/spec/frontend/invite_members/utils/response_message_parser_spec.js
@@ -2,18 +2,20 @@ import {
responseMessageFromSuccess,
responseMessageFromError,
} from '~/invite_members/utils/response_message_parser';
+import { membersApiResponse, invitationsApiResponse } from '../mock_data/api_responses';
describe('Response message parser', () => {
- const expectedMessage = 'expected display message';
+ const expectedMessage = 'expected display and message.';
describe('parse message from successful response', () => {
const exampleKeyedMsg = { 'email@example.com': expectedMessage };
+ const exampleFirstPartMultiple = 'username1: expected display and message.';
const exampleUserMsgMultiple =
- ' and username1: id not found and username2: email is restricted';
+ ' and username2: id not found and restricted email. and username3: email is restricted.';
it.each([
[[{ data: { message: expectedMessage } }]],
- [[{ data: { message: expectedMessage + exampleUserMsgMultiple } }]],
+ [[{ data: { message: exampleFirstPartMultiple + exampleUserMsgMultiple } }]],
[[{ data: { error: expectedMessage } }]],
[[{ data: { message: [expectedMessage] } }]],
[[{ data: { message: exampleKeyedMsg } }]],
@@ -33,4 +35,24 @@ describe('Response message parser', () => {
expect(responseMessageFromError(errorResponse)).toBe(expectedMessage);
});
});
+
+ describe('displaying only the first error when a response has messages for multiple users', () => {
+ const expected =
+ "The member's email address is not allowed for this project. Go to the Admin area > Sign-up restrictions, and check Allowed domains for sign-ups.";
+
+ it.each([
+ [[{ data: membersApiResponse.MULTIPLE_USERS_RESTRICTED }]],
+ [[{ data: invitationsApiResponse.MULTIPLE_EMAIL_RESTRICTED }]],
+ [[{ data: invitationsApiResponse.EMAIL_RESTRICTED }]],
+ ])(`returns "${expectedMessage}" from success response: %j`, (restrictedResponse) => {
+ expect(responseMessageFromSuccess(restrictedResponse)).toBe(expected);
+ });
+
+ it.each([[{ response: { data: membersApiResponse.SINGLE_USER_RESTRICTED } }]])(
+ `returns "${expectedMessage}" from error response: %j`,
+ (singleRestrictedResponse) => {
+ expect(responseMessageFromError(singleRestrictedResponse)).toBe(expected);
+ },
+ );
+ });
});
diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb
index 7fcf246d091..0b546ce9b29 100644
--- a/spec/models/member_spec.rb
+++ b/spec/models/member_spec.rb
@@ -65,6 +65,8 @@ RSpec.describe Member do
end
context 'with admin signup restrictions' do
+ let(:expected_message) { _('is not allowed for this group. Check with your administrator.') }
+
context 'when allowed domains for signup is enabled' do
before do
stub_application_setting(domain_allowlist: ['example.com'])
@@ -74,7 +76,7 @@ RSpec.describe Member do
member = build(:group_member, :invited, invite_email: 'info@gitlab.com')
expect(member).not_to be_valid
- expect(member.errors.messages[:user].first).to eq(_('domain is not authorized for sign-up.'))
+ expect(member.errors.messages[:user].first).to eq(expected_message)
end
end
@@ -88,7 +90,7 @@ RSpec.describe Member do
member = build(:group_member, :invited, invite_email: 'denylist@example.org')
expect(member).not_to be_valid
- expect(member.errors.messages[:user].first).to eq(_('is not from an allowed domain.'))
+ expect(member.errors.messages[:user].first).to eq(expected_message)
end
end
@@ -102,7 +104,7 @@ RSpec.describe Member do
member = build(:group_member, :invited, invite_email: 'info@gitlab.com')
expect(member).not_to be_valid
- expect(member.errors.messages[:user].first).to eq(_('is not allowed. Try again with a different email address, or contact your GitLab admin.'))
+ expect(member.errors.messages[:user].first).to eq(expected_message)
end
end
end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 1ef0d67958c..3fab026ba79 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -494,6 +494,8 @@ RSpec.describe User do
end
describe 'email' do
+ let(:expected_error) { _('is not allowed for sign-up. Check with your administrator.') }
+
context 'when no signup domains allowed' do
before do
stub_application_setting(domain_allowlist: [])
@@ -537,7 +539,7 @@ RSpec.describe User do
it 'rejects example@test.com' do
user = build(:user, email: "example@test.com")
expect(user).to be_invalid
- expect(user.errors.messages[:email].first).to eq(_('domain is not authorized for sign-up.'))
+ expect(user.errors.messages[:email].first).to eq(expected_error)
end
end
@@ -554,13 +556,13 @@ RSpec.describe User do
it 'rejects info@test.example.com' do
user = build(:user, email: "info@test.example.com")
expect(user).to be_invalid
- expect(user.errors.messages[:email].first).to eq(_('domain is not authorized for sign-up.'))
+ expect(user.errors.messages[:email].first).to eq(expected_error)
end
it 'rejects example@test.com' do
user = build(:user, email: "example@test.com")
expect(user).to be_invalid
- expect(user.errors.messages[:email].first).to eq(_('domain is not authorized for sign-up.'))
+ expect(user.errors.messages[:email].first).to eq(expected_error)
end
it 'accepts example@test.com when added by another user' do
@@ -598,7 +600,7 @@ RSpec.describe User do
it 'rejects info@example.com' do
user = build(:user, email: 'info@example.com')
expect(user).not_to be_valid
- expect(user.errors.messages[:email].first).to eq(_('is not from an allowed domain.'))
+ expect(user.errors.messages[:email].first).to eq(expected_error)
end
it 'accepts info@example.com when added by another user' do
@@ -632,7 +634,7 @@ RSpec.describe User do
it 'rejects info@example.com' do
user = build(:user, email: 'info@example.com')
expect(user).not_to be_valid
- expect(user.errors.messages[:email].first).to eq(_('domain is not authorized for sign-up.'))
+ expect(user.errors.messages[:email].first).to eq(expected_error)
end
end
end
@@ -673,7 +675,7 @@ RSpec.describe User do
user = build(:user, email: 'info@gitlab.com')
expect(user).not_to be_valid
- expect(user.errors.messages[:email].first).to eq(_('is not allowed. Try again with a different email address, or contact your GitLab admin.'))
+ expect(user.errors.messages[:email].first).to eq(expected_error)
end
it 'does accept a valid email address' do
diff --git a/spec/services/members/create_service_spec.rb b/spec/services/members/create_service_spec.rb
index 15ed5c5a33f..2e6e6041fc3 100644
--- a/spec/services/members/create_service_spec.rb
+++ b/spec/services/members/create_service_spec.rb
@@ -80,7 +80,7 @@ RSpec.describe Members::CreateService, :aggregate_failures, :clean_gitlab_redis_
it 'does not add a member' do
expect(execute_service[:status]).to eq(:error)
- expect(execute_service[:message]).to eq('Invite email has already been taken')
+ expect(execute_service[:message]).to eq("The member's email address has already been taken")
expect(OnboardingProgress.completed?(source.namespace, :user_added)).to be(false)
end
end
diff --git a/spec/services/members/invite_service_spec.rb b/spec/services/members/invite_service_spec.rb
index dd82facaf14..478733e8aa0 100644
--- a/spec/services/members/invite_service_spec.rb
+++ b/spec/services/members/invite_service_spec.rb
@@ -150,7 +150,7 @@ RSpec.describe Members::InviteService, :aggregate_failures, :clean_gitlab_redis_
expect_to_create_members(count: 1)
expect(result[:status]).to eq(:error)
expect(result[:message][invited_member.invite_email])
- .to eq("Invite email has already been taken")
+ .to eq("The member's email address has already been taken")
expect(project.users).to include project_user
end
end
diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb
index 9c0f80e174a..5a52f4fad6f 100644
--- a/spec/services/projects/transfer_service_spec.rb
+++ b/spec/services/projects/transfer_service_spec.rb
@@ -518,58 +518,30 @@ RSpec.describe Projects::TransferService do
group.add_owner(user)
end
- context 'when the feature flag `specialized_worker_for_project_transfer_auth_recalculation` is enabled' do
- before do
- stub_feature_flags(specialized_worker_for_project_transfer_auth_recalculation: true)
- end
-
- it 'calls AuthorizedProjectUpdate::ProjectRecalculateWorker to update project authorizations' do
- expect(AuthorizedProjectUpdate::ProjectRecalculateWorker)
- .to receive(:perform_async).with(project.id)
-
- execute_transfer
- end
+ it 'calls AuthorizedProjectUpdate::ProjectRecalculateWorker to update project authorizations' do
+ expect(AuthorizedProjectUpdate::ProjectRecalculateWorker)
+ .to receive(:perform_async).with(project.id)
- it 'calls AuthorizedProjectUpdate::UserRefreshFromReplicaWorker with a delay to update project authorizations' do
- user_ids = [user.id, member_of_old_group.id, member_of_new_group.id].map { |id| [id] }
-
- expect(AuthorizedProjectUpdate::UserRefreshFromReplicaWorker).to(
- receive(:bulk_perform_in)
- .with(1.hour,
- user_ids,
- batch_delay: 30.seconds, batch_size: 100)
- )
-
- subject
- end
-
- it 'refreshes the permissions of the members of the old and new namespace', :sidekiq_inline do
- expect { execute_transfer }
- .to change { member_of_old_group.authorized_projects.include?(project) }.from(true).to(false)
- .and change { member_of_new_group.authorized_projects.include?(project) }.from(false).to(true)
- end
+ execute_transfer
end
- context 'when the feature flag `specialized_worker_for_project_transfer_auth_recalculation` is disabled' do
- before do
- stub_feature_flags(specialized_worker_for_project_transfer_auth_recalculation: false)
- end
+ it 'calls AuthorizedProjectUpdate::UserRefreshFromReplicaWorker with a delay to update project authorizations' do
+ user_ids = [user.id, member_of_old_group.id, member_of_new_group.id].map { |id| [id] }
- it 'calls UserProjectAccessChangedService to update project authorizations' do
- user_ids = [user.id, member_of_old_group.id, member_of_new_group.id]
-
- expect_next_instance_of(UserProjectAccessChangedService, user_ids) do |service|
- expect(service).to receive(:execute)
- end
+ expect(AuthorizedProjectUpdate::UserRefreshFromReplicaWorker).to(
+ receive(:bulk_perform_in)
+ .with(1.hour,
+ user_ids,
+ batch_delay: 30.seconds, batch_size: 100)
+ )
- execute_transfer
- end
+ subject
+ end
- it 'refreshes the permissions of the members of the old and new namespace' do
- expect { execute_transfer }
- .to change { member_of_old_group.authorized_projects.include?(project) }.from(true).to(false)
- .and change { member_of_new_group.authorized_projects.include?(project) }.from(false).to(true)
- end
+ it 'refreshes the permissions of the members of the old and new namespace', :sidekiq_inline do
+ expect { execute_transfer }
+ .to change { member_of_old_group.authorized_projects.include?(project) }.from(true).to(false)
+ .and change { member_of_new_group.authorized_projects.include?(project) }.from(false).to(true)
end
end
diff --git a/workhorse/internal/upstream/upstream.go b/workhorse/internal/upstream/upstream.go
index d6e5e7766b5..e57f58d59dd 100644
--- a/workhorse/internal/upstream/upstream.go
+++ b/workhorse/internal/upstream/upstream.go
@@ -50,7 +50,7 @@ type upstream struct {
geoLocalRoutes []routeEntry
geoProxyCableRoute routeEntry
geoProxyRoute routeEntry
- geoProxyTestChannel chan struct{}
+ geoProxyPollSleep func(time.Duration)
accessLogger *logrus.Logger
enableGeoProxyFeature bool
mu sync.RWMutex
@@ -68,6 +68,9 @@ func newUpstream(cfg config.Config, accessLogger *logrus.Logger, routesCallback
enableGeoProxyFeature: os.Getenv("GEO_SECONDARY_PROXY") == "1",
geoProxyBackend: &url.URL{},
}
+ if up.geoProxyPollSleep == nil {
+ up.geoProxyPollSleep = time.Sleep
+ }
if up.Backend == nil {
up.Backend = DefaultBackend
}
@@ -205,13 +208,7 @@ func (u *upstream) findGeoProxyRoute(cleanedPath string, r *http.Request) *route
func (u *upstream) pollGeoProxyAPI() {
for {
u.callGeoProxyAPI()
-
- // Notify tests when callGeoProxyAPI() finishes
- if u.geoProxyTestChannel != nil {
- u.geoProxyTestChannel <- struct{}{}
- }
-
- time.Sleep(geoProxyApiPollingInterval)
+ u.geoProxyPollSleep(geoProxyApiPollingInterval)
}
}
@@ -234,6 +231,11 @@ func (u *upstream) updateGeoProxyFields(geoProxyURL *url.URL) {
defer u.mu.Unlock()
u.geoProxyBackend = geoProxyURL
+
+ if u.geoProxyBackend.String() == "" {
+ return
+ }
+
geoProxyRoundTripper := roundtripper.NewBackendRoundTripper(u.geoProxyBackend, "", u.ProxyHeadersTimeout, u.DevelopmentMode)
geoProxyUpstream := proxypkg.NewProxy(u.geoProxyBackend, u.Version, geoProxyRoundTripper)
u.geoProxyCableRoute = u.wsRoute(`^/-/cable\z`, geoProxyUpstream)
diff --git a/workhorse/internal/upstream/upstream_test.go b/workhorse/internal/upstream/upstream_test.go
index c86c03920f0..efc85dd6c2e 100644
--- a/workhorse/internal/upstream/upstream_test.go
+++ b/workhorse/internal/upstream/upstream_test.go
@@ -7,6 +7,7 @@ import (
"net/http"
"net/http/httptest"
"testing"
+ "time"
"github.com/sirupsen/logrus"
"github.com/stretchr/testify/require"
@@ -71,10 +72,10 @@ func TestGeoProxyFeatureDisabledOnGeoSecondarySite(t *testing.T) {
defer rsDeferredClose()
geoProxyEndpointResponseBody := fmt.Sprintf(`{"geo_proxy_url":"%v"}`, remoteServer.URL)
- railsServer, deferredClose := startRailsServer("Local Rails server", geoProxyEndpointResponseBody)
+ railsServer, deferredClose := startRailsServer("Local Rails server", &geoProxyEndpointResponseBody)
defer deferredClose()
- ws, wsDeferredClose := startWorkhorseServer(railsServer.URL, false)
+ ws, wsDeferredClose, _ := startWorkhorseServer(railsServer.URL, false)
defer wsDeferredClose()
testCases := []testCase{
@@ -91,10 +92,10 @@ func TestGeoProxyFeatureEnabledOnGeoSecondarySite(t *testing.T) {
defer rsDeferredClose()
geoProxyEndpointResponseBody := fmt.Sprintf(`{"geo_proxy_url":"%v"}`, remoteServer.URL)
- railsServer, deferredClose := startRailsServer("Local Rails server", geoProxyEndpointResponseBody)
+ railsServer, deferredClose := startRailsServer("Local Rails server", &geoProxyEndpointResponseBody)
defer deferredClose()
- ws, wsDeferredClose := startWorkhorseServer(railsServer.URL, true)
+ ws, wsDeferredClose, _ := startWorkhorseServer(railsServer.URL, true)
defer wsDeferredClose()
testCases := []testCase{
@@ -109,10 +110,10 @@ func TestGeoProxyFeatureEnabledOnGeoSecondarySite(t *testing.T) {
// This test can be removed when the environment variable `GEO_SECONDARY_PROXY` is removed
func TestGeoProxyFeatureDisabledOnNonGeoSecondarySite(t *testing.T) {
geoProxyEndpointResponseBody := "{}"
- railsServer, deferredClose := startRailsServer("Local Rails server", geoProxyEndpointResponseBody)
+ railsServer, deferredClose := startRailsServer("Local Rails server", &geoProxyEndpointResponseBody)
defer deferredClose()
- ws, wsDeferredClose := startWorkhorseServer(railsServer.URL, false)
+ ws, wsDeferredClose, _ := startWorkhorseServer(railsServer.URL, false)
defer wsDeferredClose()
testCases := []testCase{
@@ -126,10 +127,10 @@ func TestGeoProxyFeatureDisabledOnNonGeoSecondarySite(t *testing.T) {
func TestGeoProxyFeatureEnabledOnNonGeoSecondarySite(t *testing.T) {
geoProxyEndpointResponseBody := "{}"
- railsServer, deferredClose := startRailsServer("Local Rails server", geoProxyEndpointResponseBody)
+ railsServer, deferredClose := startRailsServer("Local Rails server", &geoProxyEndpointResponseBody)
defer deferredClose()
- ws, wsDeferredClose := startWorkhorseServer(railsServer.URL, true)
+ ws, wsDeferredClose, _ := startWorkhorseServer(railsServer.URL, true)
defer wsDeferredClose()
testCases := []testCase{
@@ -143,10 +144,10 @@ func TestGeoProxyFeatureEnabledOnNonGeoSecondarySite(t *testing.T) {
func TestGeoProxyFeatureEnabledButWithAPIError(t *testing.T) {
geoProxyEndpointResponseBody := "Invalid response"
- railsServer, deferredClose := startRailsServer("Local Rails server", geoProxyEndpointResponseBody)
+ railsServer, deferredClose := startRailsServer("Local Rails server", &geoProxyEndpointResponseBody)
defer deferredClose()
- ws, wsDeferredClose := startWorkhorseServer(railsServer.URL, true)
+ ws, wsDeferredClose, _ := startWorkhorseServer(railsServer.URL, true)
defer wsDeferredClose()
testCases := []testCase{
@@ -158,6 +159,49 @@ func TestGeoProxyFeatureEnabledButWithAPIError(t *testing.T) {
runTestCases(t, ws, testCases)
}
+func TestGeoProxyFeatureEnablingAndDisabling(t *testing.T) {
+ remoteServer, rsDeferredClose := startRemoteServer("Geo primary")
+ defer rsDeferredClose()
+
+ geoProxyEndpointEnabledResponseBody := fmt.Sprintf(`{"geo_proxy_url":"%v"}`, remoteServer.URL)
+ geoProxyEndpointDisabledResponseBody := "{}"
+ geoProxyEndpointResponseBody := geoProxyEndpointEnabledResponseBody
+
+ railsServer, deferredClose := startRailsServer("Local Rails server", &geoProxyEndpointResponseBody)
+ defer deferredClose()
+
+ ws, wsDeferredClose, waitForNextApiPoll := startWorkhorseServer(railsServer.URL, true)
+ defer wsDeferredClose()
+
+ testCasesLocal := []testCase{
+ {"jobs request is served locally", "/api/v4/jobs/request", "Local Rails server received request to path /api/v4/jobs/request"},
+ {"health check is served locally", "/-/health", "Local Rails server received request to path /-/health"},
+ {"unknown route is served locally", "/anything", "Local Rails server received request to path /anything"},
+ }
+
+ testCasesProxied := []testCase{
+ {"jobs request is forwarded", "/api/v4/jobs/request", "Geo primary received request to path /api/v4/jobs/request"},
+ {"health check is served locally", "/-/health", "Local Rails server received request to path /-/health"},
+ {"unknown route is forwarded", "/anything", "Geo primary received request to path /anything"},
+ }
+
+ // Enabled initially, run tests
+ runTestCases(t, ws, testCasesProxied)
+
+ // Disable proxying and run tests. It's safe to write to
+ // geoProxyEndpointResponseBody because the polling goroutine is blocked.
+ geoProxyEndpointResponseBody = geoProxyEndpointDisabledResponseBody
+ waitForNextApiPoll()
+
+ runTestCases(t, ws, testCasesLocal)
+
+ // Re-enable proxying and run tests
+ geoProxyEndpointResponseBody = geoProxyEndpointEnabledResponseBody
+ waitForNextApiPoll()
+
+ runTestCases(t, ws, testCasesProxied)
+}
+
func runTestCases(t *testing.T, ws *httptest.Server, testCases []testCase) {
t.Helper()
for _, tc := range testCases {
@@ -195,13 +239,13 @@ func startRemoteServer(serverName string) (*httptest.Server, func()) {
return ts, ts.Close
}
-func startRailsServer(railsServerName string, geoProxyEndpointResponseBody string) (*httptest.Server, func()) {
+func startRailsServer(railsServerName string, geoProxyEndpointResponseBody *string) (*httptest.Server, func()) {
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
var body string
if r.URL.Path == geoProxyEndpoint {
w.Header().Set("Content-Type", "application/vnd.gitlab-workhorse+json")
- body = geoProxyEndpointResponseBody
+ body = *geoProxyEndpointResponseBody
} else {
body = railsServerName + " received request to path " + r.URL.Path
}
@@ -213,15 +257,19 @@ func startRailsServer(railsServerName string, geoProxyEndpointResponseBody strin
return ts, ts.Close
}
-func startWorkhorseServer(railsServerURL string, enableGeoProxyFeature bool) (*httptest.Server, func()) {
- geoProxyTestChannel := make(chan struct{})
+func startWorkhorseServer(railsServerURL string, enableGeoProxyFeature bool) (*httptest.Server, func(), func()) {
+ geoProxySleepC := make(chan struct{})
+ geoProxySleep := func(time.Duration) {
+ geoProxySleepC <- struct{}{}
+ <-geoProxySleepC
+ }
myConfigureRoutes := func(u *upstream) {
// Enable environment variable "feature flag"
u.enableGeoProxyFeature = enableGeoProxyFeature
- // An empty message will be sent to this channel after every callGeoProxyAPI()
- u.geoProxyTestChannel = geoProxyTestChannel
+ // Replace the time.Sleep function with geoProxySleep
+ u.geoProxyPollSleep = geoProxySleep
// call original
configureRoutes(u)
@@ -231,13 +279,20 @@ func startWorkhorseServer(railsServerURL string, enableGeoProxyFeature bool) (*h
ws := httptest.NewServer(upstreamHandler)
testhelper.ConfigureSecret()
+ waitForNextApiPoll := func() {}
+
if enableGeoProxyFeature {
- // Wait for an empty message from callGeoProxyAPI(). This should be done on
- // all tests where enableGeoProxyFeature is true, including the ones where
- // we expect geoProxyURL to be nil or error, to ensure the tests do not pass
- // by coincidence.
- <-geoProxyTestChannel
+ // Wait for geoProxySleep to be entered for the first time
+ <-geoProxySleepC
+
+ waitForNextApiPoll = func() {
+ // Cause geoProxySleep to return
+ geoProxySleepC <- struct{}{}
+
+ // Wait for geoProxySleep to be entered again
+ <-geoProxySleepC
+ }
}
- return ws, ws.Close
+ return ws, ws.Close, waitForNextApiPoll
}
View Lorry Controller Status