diff options
46 files changed, 1453 insertions, 999 deletions
diff --git a/app/assets/javascripts/diffs/components/diff_file.vue b/app/assets/javascripts/diffs/components/diff_file.vue index e09bf1f8005..8babc05f1ce 100644 --- a/app/assets/javascripts/diffs/components/diff_file.vue +++ b/app/assets/javascripts/diffs/components/diff_file.vue @@ -58,9 +58,6 @@ export default { hasDiff() { return hasDiff(this.file); }, - isActive() { - return this.currentDiffFileId === this.file.file_hash; - }, isFileTooLarge() { return this.file.viewer.error === diffViewerErrors.too_large; }, @@ -146,7 +143,7 @@ export default { <div :id="file.file_hash" :class="{ - 'is-active': isActive, + 'is-active': currentDiffFileId === file.file_hash, }" class="diff-file file-holder" > @@ -156,7 +153,6 @@ export default { :collapsible="true" :expanded="!isCollapsed" :add-merge-request-buttons="true" - :is-active="isActive" class="js-file-title file-title" @toggleFile="handleToggle" @showForkMessage="showForkMessage" diff --git a/app/assets/javascripts/diffs/components/diff_file_header.vue b/app/assets/javascripts/diffs/components/diff_file_header.vue index bad82c84601..d4270960f57 100644 --- a/app/assets/javascripts/diffs/components/diff_file_header.vue +++ b/app/assets/javascripts/diffs/components/diff_file_header.vue @@ -55,11 +55,6 @@ export default { type: Boolean, required: true, }, - isActive: { - type: Boolean, - required: false, - default: false, - }, }, computed: { ...mapGetters('diffs', ['diffHasExpandedDiscussions', 'diffHasDiscussions']), @@ -163,9 +158,6 @@ export default { <div ref="header" class="js-file-title file-title file-title-flex-parent" - :class="{ - 'is-active': isActive, - }" @click.self="handleToggleFile" > <div class="file-header-content"> diff --git a/app/assets/javascripts/diffs/components/tree_list.vue b/app/assets/javascripts/diffs/components/tree_list.vue index fcf87807e75..eca9091f92f 100644 --- a/app/assets/javascripts/diffs/components/tree_list.vue +++ b/app/assets/javascripts/diffs/components/tree_list.vue @@ -26,7 +26,7 @@ export default { }; }, computed: { - ...mapState('diffs', ['tree', 'renderTreeList', 'currentDiffFileId', 'viewedDiffFileIds']), + ...mapState('diffs', ['tree', 'renderTreeList']), ...mapGetters('diffs', ['allBlobs']), filteredTreeList() { const search = this.search.toLowerCase().trim(); @@ -96,8 +96,6 @@ export default { :level="0" :hide-file-stats="hideFileStats" :file-row-component="$options.DiffFileRow" - :active-file="currentDiffFileId" - :viewed-files="viewedDiffFileIds" @toggleTreeOpen="toggleTreeOpen" @clickFile="scrollToFile" /> diff --git a/app/assets/javascripts/diffs/store/modules/diff_state.js b/app/assets/javascripts/diffs/store/modules/diff_state.js index f1bd9d8981d..81f1506260c 100644 --- a/app/assets/javascripts/diffs/store/modules/diff_state.js +++ b/app/assets/javascripts/diffs/store/modules/diff_state.js @@ -26,7 +26,6 @@ export default () => ({ showTreeList: true, currentDiffFileId: '', projectPath: '', - viewedDiffFileIds: [], commentForms: [], highlightedRow: null, renderTreeList: true, diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js index 6438dad4f7f..bb4c80b5759 100644 --- a/app/assets/javascripts/diffs/store/mutations.js +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -284,9 +284,6 @@ export default { }, [types.UPDATE_CURRENT_DIFF_FILE_ID](state, fileId) { state.currentDiffFileId = fileId; - if (!state.viewedDiffFileIds.includes(fileId)) { - state.viewedDiffFileIds = [fileId, ...state.viewedDiffFileIds]; - } }, [types.OPEN_DIFF_FILE_COMMENT_FORM](state, formData) { state.commentForms.push({ diff --git a/app/assets/javascripts/releases/mount_edit.js b/app/assets/javascripts/releases/mount_edit.js index 102c4367aac..44530e4961a 100644 --- a/app/assets/javascripts/releases/mount_edit.js +++ b/app/assets/javascripts/releases/mount_edit.js @@ -1,22 +1,20 @@ import Vue from 'vue'; import ReleaseEditApp from './components/app_edit.vue'; import createStore from './stores'; -import detailModule from './stores/modules/detail'; +import createDetailModule from './stores/modules/detail'; export default () => { const el = document.getElementById('js-edit-release-page'); const store = createStore({ modules: { - detail: detailModule, + detail: createDetailModule(el.dataset), }, featureFlags: { releaseShowPage: Boolean(gon.features?.releaseShowPage), }, }); - store.dispatch('detail/setInitialState', el.dataset); - return new Vue({ el, store, diff --git a/app/assets/javascripts/releases/mount_show.js b/app/assets/javascripts/releases/mount_show.js index 73e34869b21..7ddc8e786c1 100644 --- a/app/assets/javascripts/releases/mount_show.js +++ b/app/assets/javascripts/releases/mount_show.js @@ -1,17 +1,16 @@ import Vue from 'vue'; import ReleaseShowApp from './components/app_show.vue'; import createStore from './stores'; -import detailModule from './stores/modules/detail'; +import createDetailModule from './stores/modules/detail'; export default () => { const el = document.getElementById('js-show-release-page'); const store = createStore({ modules: { - detail: detailModule, + detail: createDetailModule(el.dataset), }, }); - store.dispatch('detail/setInitialState', el.dataset); return new Vue({ el, diff --git a/app/assets/javascripts/releases/stores/modules/detail/actions.js b/app/assets/javascripts/releases/stores/modules/detail/actions.js index 35901a654b0..1b77f01368e 100644 --- a/app/assets/javascripts/releases/stores/modules/detail/actions.js +++ b/app/assets/javascripts/releases/stores/modules/detail/actions.js @@ -5,9 +5,6 @@ import { s__ } from '~/locale'; import { redirectTo } from '~/lib/utils/url_utility'; import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; -export const setInitialState = ({ commit }, initialState) => - commit(types.SET_INITIAL_STATE, initialState); - export const requestRelease = ({ commit }) => commit(types.REQUEST_RELEASE); export const receiveReleaseSuccess = ({ commit }, data) => commit(types.RECEIVE_RELEASE_SUCCESS, data); diff --git a/app/assets/javascripts/releases/stores/modules/detail/index.js b/app/assets/javascripts/releases/stores/modules/detail/index.js index 243c2389d8c..b4430cff2ab 100644 --- a/app/assets/javascripts/releases/stores/modules/detail/index.js +++ b/app/assets/javascripts/releases/stores/modules/detail/index.js @@ -1,10 +1,10 @@ import * as actions from './actions'; import mutations from './mutations'; -import state from './state'; +import createState from './state'; -export default { +export default initialState => ({ namespaced: true, actions, mutations, - state, -}; + state: createState(initialState), +}); diff --git a/app/assets/javascripts/releases/stores/modules/detail/mutation_types.js b/app/assets/javascripts/releases/stores/modules/detail/mutation_types.js index 75e1d78a645..51c0590012a 100644 --- a/app/assets/javascripts/releases/stores/modules/detail/mutation_types.js +++ b/app/assets/javascripts/releases/stores/modules/detail/mutation_types.js @@ -1,5 +1,3 @@ -export const SET_INITIAL_STATE = 'SET_INITIAL_STATE'; - export const REQUEST_RELEASE = 'REQUEST_RELEASE'; export const RECEIVE_RELEASE_SUCCESS = 'RECEIVE_RELEASE_SUCCESS'; export const RECEIVE_RELEASE_ERROR = 'RECEIVE_RELEASE_ERROR'; diff --git a/app/assets/javascripts/releases/stores/modules/detail/mutations.js b/app/assets/javascripts/releases/stores/modules/detail/mutations.js index d739978d755..913db6c2b2a 100644 --- a/app/assets/javascripts/releases/stores/modules/detail/mutations.js +++ b/app/assets/javascripts/releases/stores/modules/detail/mutations.js @@ -1,12 +1,6 @@ import * as types from './mutation_types'; export default { - [types.SET_INITIAL_STATE](state, initialState) { - Object.keys(state).forEach(key => { - state[key] = initialState[key]; - }); - }, - [types.REQUEST_RELEASE](state) { state.isFetchingRelease = true; }, diff --git a/app/assets/javascripts/releases/stores/modules/detail/state.js b/app/assets/javascripts/releases/stores/modules/detail/state.js index 7e3d975f1ae..a19e8d044e2 100644 --- a/app/assets/javascripts/releases/stores/modules/detail/state.js +++ b/app/assets/javascripts/releases/stores/modules/detail/state.js @@ -1,10 +1,17 @@ -export default () => ({ - projectId: null, - tagName: null, - releasesPagePath: null, - markdownDocsPath: null, - markdownPreviewPath: null, - updateReleaseApiDocsPath: null, +export default ({ + projectId, + tagName, + releasesPagePath, + markdownDocsPath, + markdownPreviewPath, + updateReleaseApiDocsPath, +}) => ({ + projectId, + tagName, + releasesPagePath, + markdownDocsPath, + markdownPreviewPath, + updateReleaseApiDocsPath, release: null, diff --git a/app/assets/javascripts/vue_shared/components/file_row.vue b/app/assets/javascripts/vue_shared/components/file_row.vue index 5a953b351f9..4d60cf5b1cc 100644 --- a/app/assets/javascripts/vue_shared/components/file_row.vue +++ b/app/assets/javascripts/vue_shared/components/file_row.vue @@ -18,16 +18,6 @@ export default { type: Number, required: true, }, - activeFile: { - type: String, - required: false, - default: '', - }, - viewedFiles: { - type: Array, - required: false, - default: () => [], - }, }, computed: { isTree() { @@ -44,8 +34,8 @@ export default { fileClass() { return { 'file-open': this.isBlob && this.file.opened, - 'is-active': this.isBlob && (this.file.active || this.activeFile === this.file.fileHash), - 'is-viewed': this.isBlob && this.viewedFiles.includes(this.file.fileHash), + 'is-active': this.isBlob && this.file.active, + folder: this.isTree, 'is-open': this.file.opened, }; }, @@ -117,23 +107,15 @@ export default { v-else :class="fileClass" :title="file.name" - class="file-row text-left px-1 py-2 ml-n2 d-flex align-items-center" + class="file-row" role="button" @click="clickFile" @mouseleave="$emit('mouseleave', $event)" > - <div class="file-row-name-container w-100 d-flex align-items-center"> - <span - ref="textOutput" - :style="levelIndentation" - class="file-row-name str-truncated d-inline-block" - :class="[ - { 'folder font-weight-normal': isTree }, - fileClass['is-viewed'] ? 'font-weight-normal' : 'font-weight-bold', - ]" - > + <div class="file-row-name-container"> + <span ref="textOutput" :style="levelIndentation" class="file-row-name str-truncated"> <file-icon - class="file-row-icon align-middle mr-1" + class="file-row-icon" :class="{ 'text-secondary': file.type === 'tree' }" :file-name="file.name" :loading="file.loading" @@ -150,8 +132,14 @@ export default { <style> .file-row { + display: flex; + align-items: center; height: 32px; + padding: 4px 8px; + margin-left: -8px; + margin-right: -8px; border-radius: 3px; + text-align: left; cursor: pointer; } @@ -169,15 +157,24 @@ export default { } .file-row-name-container { + display: flex; + width: 100%; + align-items: center; overflow: visible; } .file-row-name { + display: inline-block; flex: 1; max-width: inherit; - height: 20px; + height: 19px; line-height: 16px; text-overflow: ellipsis; white-space: nowrap; } + +.file-row-name .file-row-icon { + margin-right: 2px; + vertical-align: middle; +} </style> diff --git a/app/assets/stylesheets/pages/diff.scss b/app/assets/stylesheets/pages/diff.scss index 991ee841398..0c043e4f3fb 100644 --- a/app/assets/stylesheets/pages/diff.scss +++ b/app/assets/stylesheets/pages/diff.scss @@ -69,11 +69,6 @@ } } - .file-title.is-active, - .file-title-flex-parent.is-active { - background-color: $gray-200; - } - @media (min-width: map-get($grid-breakpoints, md)) { &.conflict .file-title, &.conflict .file-title-flex-parent { diff --git a/app/models/x509_issuer.rb b/app/models/x509_issuer.rb index 514b38808ef..4b75e38bbde 100644 --- a/app/models/x509_issuer.rb +++ b/app/models/x509_issuer.rb @@ -7,7 +7,7 @@ class X509Issuer < ApplicationRecord validates :subject_key_identifier, presence: true, format: { with: /\A(\h{2}:){19}\h{2}\z/ } # rfc 5280 - 4.1.2.4 Issuer validates :subject, presence: true - # rfc 5280 - 4.2.1.14 CRL Distribution Points + # rfc 5280 - 4.2.1.13 CRL Distribution Points # cRLDistributionPoints extension using URI:http validates :crl_url, presence: true, public_url: true diff --git a/changelogs/unreleased/24629.yml b/changelogs/unreleased/24629.yml deleted file mode 100644 index 1b39cff8d18..00000000000 --- a/changelogs/unreleased/24629.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Highlight currently focused/viewed file in file tree -merge_request: 27703 -author: -type: changed diff --git a/changelogs/unreleased/application-limits-with-defaults.yml b/changelogs/unreleased/application-limits-with-defaults.yml new file mode 100644 index 00000000000..49281806cba --- /dev/null +++ b/changelogs/unreleased/application-limits-with-defaults.yml @@ -0,0 +1,5 @@ +--- +title: Update ApplicationLimits to prefer defaults +merge_request: 27574 +author: +type: changed diff --git a/changelogs/unreleased/refactor-x509-commit-to-signature.yml b/changelogs/unreleased/refactor-x509-commit-to-signature.yml new file mode 100644 index 00000000000..a8ac00d84a7 --- /dev/null +++ b/changelogs/unreleased/refactor-x509-commit-to-signature.yml @@ -0,0 +1,5 @@ +--- +title: Extract X509::Signature from X509::Commit +merge_request: 27327 +author: Roger Meier +type: changed diff --git a/db/migrate/20200319123041_update_plan_limits_defaults.rb b/db/migrate/20200319123041_update_plan_limits_defaults.rb new file mode 100644 index 00000000000..252af26cb7f --- /dev/null +++ b/db/migrate/20200319123041_update_plan_limits_defaults.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class UpdatePlanLimitsDefaults < ActiveRecord::Migration[6.0] + DOWNTIME = false + + def up + change_column_default :plan_limits, :project_hooks, 100 + change_column_default :plan_limits, :group_hooks, 50 + change_column_default :plan_limits, :ci_project_subscriptions, 2 + change_column_default :plan_limits, :ci_pipeline_schedules, 10 + end + + def down + change_column_default :plan_limits, :project_hooks, 0 + change_column_default :plan_limits, :group_hooks, 0 + change_column_default :plan_limits, :ci_project_subscriptions, 0 + change_column_default :plan_limits, :ci_pipeline_schedules, 0 + end +end diff --git a/db/structure.sql b/db/structure.sql index ee6a249093d..af6fa7555bb 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -4508,10 +4508,10 @@ CREATE TABLE public.plan_limits ( ci_active_pipelines integer DEFAULT 0 NOT NULL, ci_pipeline_size integer DEFAULT 0 NOT NULL, ci_active_jobs integer DEFAULT 0 NOT NULL, - project_hooks integer DEFAULT 0 NOT NULL, - group_hooks integer DEFAULT 0 NOT NULL, - ci_project_subscriptions integer DEFAULT 0 NOT NULL, - ci_pipeline_schedules integer DEFAULT 0 NOT NULL + project_hooks integer DEFAULT 100 NOT NULL, + group_hooks integer DEFAULT 50 NOT NULL, + ci_project_subscriptions integer DEFAULT 2 NOT NULL, + ci_pipeline_schedules integer DEFAULT 10 NOT NULL ); CREATE SEQUENCE public.plan_limits_id_seq @@ -12747,6 +12747,7 @@ INSERT INTO "schema_migrations" (version) VALUES ('20200318164448'), ('20200318165448'), ('20200318175008'), +('20200319123041'), ('20200319203901'), ('20200323075043'), ('20200323122201'); diff --git a/doc/administration/auth/ldap-ee.md b/doc/administration/auth/ldap-ee.md index c5e4bee3c58..655e9df6f76 100644 --- a/doc/administration/auth/ldap-ee.md +++ b/doc/administration/auth/ldap-ee.md @@ -405,198 +405,4 @@ network and LDAP server response time will affect these metrics. ## Troubleshooting -### Referral error - -If you see `LDAP search error: Referral` in the logs, or when troubleshooting -LDAP Group Sync, this error may indicate a configuration problem. The LDAP -configuration `/etc/gitlab/gitlab.rb` (Omnibus) or `config/gitlab.yml` (source) -is in YAML format and is sensitive to indentation. Check that `group_base` and -`admin_group` configuration keys are indented 2 spaces past the server -identifier. The default identifier is `main` and an example snippet looks like -the following: - -```yaml -main: # 'main' is the GitLab 'provider ID' of this LDAP server - label: 'LDAP' - host: 'ldap.example.com' - ... - group_base: 'cn=my_group,ou=groups,dc=example,dc=com' - admin_group: 'my_admin_group' -``` - -[reconfigure]: ../restart_gitlab.md#omnibus-gitlab-reconfigure -[restart]: ../restart_gitlab.md#installations-from-source - -[^1]: In Active Directory, a user is marked as disabled/blocked if the user - account control attribute (`userAccountControl:1.2.840.113556.1.4.803`) - has bit 2 set. See <https://ctovswild.com/2009/09/03/bitmask-searches-in-ldap/> - for more information. - -### User DN has changed - -When an LDAP user is created in GitLab, their LDAP DN is stored for later reference. - -If GitLab cannot find a user by their DN, it will attempt to fallback -to finding the user by their email. If the lookup is successful, GitLab will -update the stored DN to the new value. - -### User is not being added to a group - -Sometimes you may think a particular user should be added to a GitLab group via -LDAP group sync, but for some reason it's not happening. There are several -things to check to debug the situation. - -- Ensure LDAP configuration has a `group_base` specified. This configuration is - required for group sync to work properly. -- Ensure the correct LDAP group link is added to the GitLab group. Check group - links by visiting the GitLab group, then **Settings dropdown > LDAP groups**. -- Check that the user has an LDAP identity: - 1. Sign in to GitLab as an administrator user. - 1. Navigate to **Admin Area > Users**. - 1. Search for the user - 1. Open the user, by clicking on their name. Do not click 'Edit'. - 1. Navigate to the **Identities** tab. There should be an LDAP identity with - an LDAP DN as the 'Identifier'. - -If all of the above looks good, jump in to a little more advanced debugging. -Often, the best way to learn more about why group sync is behaving a certain -way is to enable debug logging. There is verbose output that details every -step of the sync. - -1. Start a Rails console: - - ```shell - # For Omnibus installations - sudo gitlab-rails console - - # For installations from source - sudo -u git -H bundle exec rails console -e production - ``` - -1. Set the log level to debug (only for this session): - - ```ruby - Rails.logger.level = Logger::DEBUG - ``` - -1. Choose a GitLab group to test with. This group should have an LDAP group link - already configured. If the output is `nil`, the group could not be found. - If a bunch of group attributes are output, your group was found successfully. - - ```ruby - group = Group.find_by(name: 'my_group') - - # Output - => #<Group:0x007fe825196558 id: 1234, name: "my_group"...> - ``` - -1. Run a group sync for this particular group. - - ```ruby - EE::Gitlab::Auth::Ldap::Sync::Group.execute_all_providers(group) - ``` - -1. Look through the output of the sync. See [example log output](#example-log-output) - below for more information about the output. -1. If you still aren't able to see why the user isn't being added, query the - LDAP group directly to see what members are listed. Still in the Rails console, - run the following query: - - ```ruby - adapter = Gitlab::Auth::Ldap::Adapter.new('ldapmain') # If `main` is the LDAP provider - ldap_group = EE::Gitlab::Auth::Ldap::Group.find_by_cn('group_cn_here', adapter) - - # Output - => #<EE::Gitlab::Auth::Ldap::Group:0x007fcbdd0bb6d8 - ``` - -1. Query the LDAP group's member DNs and see if the user's DN is in the list. - One of the DNs here should match the 'Identifier' from the LDAP identity - checked earlier. If it doesn't, the user does not appear to be in the LDAP - group. - - ```ruby - ldap_group.member_dns - - # Output - => ["uid=john,ou=people,dc=example,dc=com", "uid=mary,ou=people,dc=example,dc=com"] - ``` - -1. Some LDAP servers don't store members by DN. Rather, they use UIDs instead. - If you didn't see results from the last query, try querying by UIDs instead. - - ```ruby - ldap_group.member_uids - - # Output - => ['john','mary'] - ``` - -#### Example log output - -The output of the last command will be very verbose, but contains lots of -helpful information. For the most part you can ignore log entries that are SQL -statements. - -Indicates the point where syncing actually begins: - -```shell -Started syncing all providers for 'my_group' group -``` - -The follow entry shows an array of all user DNs GitLab sees in the LDAP server. -Note that these are the users for a single LDAP group, not a GitLab group. If -you have multiple LDAP groups linked to this GitLab group, you will see multiple -log entries like this - one for each LDAP group. If you don't see an LDAP user -DN in this log entry, LDAP is not returning the user when we do the lookup. -Verify the user is actually in the LDAP group. - -```shell -Members in 'ldap_group_1' LDAP group: ["uid=john0,ou=people,dc=example,dc=com", -"uid=mary0,ou=people,dc=example,dc=com", "uid=john1,ou=people,dc=example,dc=com", -"uid=mary1,ou=people,dc=example,dc=com", "uid=john2,ou=people,dc=example,dc=com", -"uid=mary2,ou=people,dc=example,dc=com", "uid=john3,ou=people,dc=example,dc=com", -"uid=mary3,ou=people,dc=example,dc=com", "uid=john4,ou=people,dc=example,dc=com", -"uid=mary4,ou=people,dc=example,dc=com"] -``` - -Shortly after each of the above entries, you will see a hash of resolved member -access levels. This hash represents all user DNs GitLab thinks should have -access to this group, and at which access level (role). This hash is additive, -and more DNs may be added, or existing entries modified, based on additional -LDAP group lookups. The very last occurrence of this entry should indicate -exactly which users GitLab believes should be added to the group. - -NOTE: **Note:** -10 is 'Guest', 20 is 'Reporter', 30 is 'Developer', 40 is 'Maintainer' -and 50 is 'Owner'. - -```shell -Resolved 'my_group' group member access: {"uid=john0,ou=people,dc=example,dc=com"=>30, -"uid=mary0,ou=people,dc=example,dc=com"=>30, "uid=john1,ou=people,dc=example,dc=com"=>30, -"uid=mary1,ou=people,dc=example,dc=com"=>30, "uid=john2,ou=people,dc=example,dc=com"=>30, -"uid=mary2,ou=people,dc=example,dc=com"=>30, "uid=john3,ou=people,dc=example,dc=com"=>30, -"uid=mary3,ou=people,dc=example,dc=com"=>30, "uid=john4,ou=people,dc=example,dc=com"=>30, -"uid=mary4,ou=people,dc=example,dc=com"=>30} -``` - -It's not uncommon to see warnings like the following. These indicate that GitLab -would have added the user to a group, but the user could not be found in GitLab. -Usually this is not a cause for concern. - -If you think a particular user should already exist in GitLab, but you're seeing -this entry, it could be due to a mismatched DN stored in GitLab. See -[User DN has changed](#User-DN-has-changed) to update the user's LDAP identity. - -```shell -User with DN `uid=john0,ou=people,dc=example,dc=com` should have access -to 'my_group' group but there is no user in GitLab with that -identity. Membership will be updated once the user signs in for -the first time. -``` - -Finally, the following entry says syncing has finished for this group: - -```shell -Finished syncing all providers for 'my_group' group -``` +Please see our [administrator guide to troubleshooting LDAP](ldap-troubleshooting.md). diff --git a/doc/administration/auth/ldap-troubleshooting.md b/doc/administration/auth/ldap-troubleshooting.md new file mode 100644 index 00000000000..c97231793db --- /dev/null +++ b/doc/administration/auth/ldap-troubleshooting.md @@ -0,0 +1,672 @@ +# LDAP Troubleshooting for Administrators + +## Common Problems & Workflows + +### Connection + +#### Connection refused + +If you are getting `Connection Refused` errors when trying to connect to the +LDAP server please double-check the LDAP `port` and `encryption` settings used by +GitLab. Common combinations are `encryption: 'plain'` and `port: 389`, OR +`encryption: 'simple_tls'` and `port: 636`. + +#### Connection times out + +If GitLab cannot reach your LDAP endpoint, you will see a message like this: + +```plaintext +Could not authenticate you from Ldapmain because "Connection timed out - user specified timeout". +``` + +If your configured LDAP provider and/or endpoint is offline or otherwise +unreachable by GitLab, no LDAP user will be able to authenticate and log in. +GitLab does not cache or store credentials for LDAP users to provide authentication +during an LDAP outage. + +Contact your LDAP provider or administrator if you are seeing this error. + +#### Referral error + +If you see `LDAP search error: Referral` in the logs, or when troubleshooting +LDAP Group Sync, this error may indicate a configuration problem. The LDAP +configuration `/etc/gitlab/gitlab.rb` (Omnibus) or `config/gitlab.yml` (source) +is in YAML format and is sensitive to indentation. Check that `group_base` and +`admin_group` configuration keys are indented 2 spaces past the server +identifier. The default identifier is `main` and an example snippet looks like +the following: + +```yaml +main: # 'main' is the GitLab 'provider ID' of this LDAP server + label: 'LDAP' + host: 'ldap.example.com' + ... + group_base: 'cn=my_group,ou=groups,dc=example,dc=com' + admin_group: 'my_admin_group' +``` + +#### Query LDAP **(STARTER ONLY)** + +The following allows you to perform a search in LDAP using the rails console. +Depending on what you're trying to do, it may make more sense to query [a +user](#query-a-user-in-ldap) or [a group](#query-a-group-in-ldap-starter-only) directly, or +even [use `ldapsearch`](#ldapsearch) instead. + +```ruby +adapter = Gitlab::Auth::Ldap::Adapter.new('ldapmain') +options = { + # :base is required + # use .base or .group_base + base: adapter.config.group_base, + + # :filter is optional + # 'cn' looks for all "cn"s under :base + # '*' is the search string - here, it's a wildcard + filter: Net::Ldap::Filter.eq('cn', '*'), + + # :attributes is optional + # the attributes we want to get returned + attributes: %w(dn cn memberuid member submember uniquemember memberof) +} +adapter.ldap_search(options) +``` + +For examples of how this is run, +[review the `Adapter` module](https://gitlab.com/gitlab-org/gitlab/blob/master/ee/lib/ee/gitlab/auth/ldap/adapter.rb). + +### User logins + +#### No users are found + +If [you've confirmed](#ldap-check) that a connection to LDAP can be +established but GitLab doesn't show you LDAP users in the output, one of the +following is most likely true: + +- The `bind_dn` user doesn't have enough permissions to traverse the user tree. +- The user(s) don't fall under the [configured `base`](ldap.md#configuration). +- The [configured `user_filter`][user-filter] blocks access to the user(s). + +In this case, you con confirm which of the above is true using +[ldapsearch](#ldapsearch) with the existing LDAP configuration in your +`/etc/gitlab/gitlab.rb`. + +#### User(s) cannot login + +A user can have trouble logging in for any number of reasons. To get started, +here are some questions to ask yourself: + +- Does the user fall under the [configured `base`](ldap.md#configuration) in + LDAP? The user must fall under this `base` to login. +- Does the user pass through the [configured `user_filter`][user-filter]? + If one is not configured, this question can be ignored. If it is, then the + user must also pass through this filter to be allowed to login. + - Refer to our docs on [debugging the `user_filter`](#debug-ldap-user-filter). + +If the above are both okay, the next place to look for the problem is +the logs themselves while reproducing the issue. + +- Ask the user to login and let it fail. +- [Look through the output](#gitlab-logs) for any errors or other + messages about the login. You may see one of the other error messages on + this page, in which case that section can help resolve the issue. + +If the logs don't lead to the root of the problem, use the +[rails console](#rails-console) to [query this user](#query-a-user-in-ldap) +to see if GitLab can read this user on the LDAP server. + +It can also be helpful to +[debug a user sync](#sync-all-users-starter-only) to +investigate further. + +#### Invalid credentials on login + +If that the login credentials used are accurate on LDAP, ensure the following +are true for the user in question: + +- Make sure the user you are binding with has enough permissions to read the user's + tree and traverse it. +- Check that the `user_filter` is not blocking otherwise valid users. +- Run [an LDAP check command](#ldap-check) to make sure that the LDAP settings + are correct and [GitLab can see your users](#no-users-are-found). + +#### Email has already been taken + +A user tries to login with the correct LDAP credentials, is denied access, +and the [production.log][production-log] shows an error that looks like this: + +```plaintext +(LDAP) Error saving user <USER DN> (email@example.com): ["Email has already been taken"] +``` + +This error is referring to the email address in LDAP, `email@example.com`. Email +addresses must be unique in GitLab and LDAP links to a user's primary email (as opposed +to any of their possibly-numerous secondary emails). Another user (or even the +same user) has the email `email@example.com` set as a secondary email, which +is throwing this error. + +We can check where this conflicting email address is coming from using the +[rails console](#rails-console). Once in the console, run the following: + +```ruby +# This searches for an email among the primary AND secondary emails +user = User.find_by_any_email('email@example.com') +user.username +``` + +This will show you which user has this email address. One of two steps will +have to be taken here: + +- To create a new GitLab user/username for this user when logging in with LDAP, + remove the secondary email to remove the conflict. +- To use an existing GitLab user/username for this user to use with LDAP, + remove this email as a secondary email and make it a primary one so GitLab + will associate this profile to the LDAP identity. + +The user can do either of these steps [in their +profile](../../user/profile/index.md#user-profile) or an admin can do it. + +#### Debug LDAP user filter + +[`ldapsearch`](#ldapsearch) allows you to test your configured +[user filter][user-filter] +to confirm that it returns the users you expect it to return. + +```shell +ldapsearch -H ldaps://$host:$port -D "$bind_dn" -y bind_dn_password.txt -b "$base" "$user_filter" sAMAccountName +``` + +- Variables beginning with a `$` refer to a variable from the LDAP section of + your configuration file. +- Replace `ldaps://` with `ldap://` if you are using the plain authentication method. + Port `389` is the default `ldap://` port and `636` is the default `ldaps://` + port. +- We are assuming the password for the `bind_dn` user is in `bind_dn_password.txt`. + +#### Sync all users **(STARTER ONLY)** + +The output from a manual [user sync][user-sync] can show you what happens when +GitLab tries to sync its users against LDAP. Enter the [rails console](#rails-console) +and then run: + +```ruby +Rails.logger.level = Logger::DEBUG + +LdapSyncWorker.new.perform +``` + +Next, [learn how to read the +output](#example-console-output-after-a-user-sync-starter-only). + +##### Example console output after a user sync **(STARTER ONLY)** + +The output from a [manual user sync](#sync-all-users-starter-only) will be very verbose, and a +single user's successful sync can look like this: + +```shell +Syncing user John, email@example.com + Identity Load (0.9ms) SELECT "identities".* FROM "identities" WHERE "identities"."user_id" = 20 AND (provider LIKE 'ldap%') LIMIT 1 +Instantiating Gitlab::Auth::Ldap::Person with LDIF: +dn: cn=John Smith,ou=people,dc=example,dc=com +cn: John Smith +mail: email@example.com +memberof: cn=admin_staff,ou=people,dc=example,dc=com +uid: John + + UserSyncedAttributesMetadata Load (0.9ms) SELECT "user_synced_attributes_metadata".* FROM "user_synced_attributes_metadata" WHERE "user_synced_attributes_metadata"."user_id" = 20 LIMIT 1 + (0.3ms) BEGIN + Namespace Load (1.0ms) SELECT "namespaces".* FROM "namespaces" WHERE "namespaces"."owner_id" = 20 AND "namespaces"."type" IS NULL LIMIT 1 + Route Load (0.8ms) SELECT "routes".* FROM "routes" WHERE "routes"."source_id" = 27 AND "routes"."source_type" = 'Namespace' LIMIT 1 + Ci::Runner Load (1.1ms) SELECT "ci_runners".* FROM "ci_runners" INNER JOIN "ci_runner_namespaces" ON "ci_runners"."id" = "ci_runner_namespaces"."runner_id" WHERE "ci_runner_namespaces"."namespace_id" = 27 + (0.7ms) COMMIT + (0.4ms) BEGIN + Route Load (0.8ms) SELECT "routes".* FROM "routes" WHERE (LOWER("routes"."path") = LOWER('John')) + Namespace Load (1.0ms) SELECT "namespaces".* FROM "namespaces" WHERE "namespaces"."id" = 27 LIMIT 1 + Route Exists (0.9ms) SELECT 1 AS one FROM "routes" WHERE LOWER("routes"."path") = LOWER('John') AND "routes"."id" != 50 LIMIT 1 + User Update (1.1ms) UPDATE "users" SET "updated_at" = '2019-10-17 14:40:59.751685', "last_credential_check_at" = '2019-10-17 14:40:59.738714' WHERE "users"."id" = 20 +``` + +There's a lot here, so let's go over what could be helpful when debugging. + +First, GitLab will look for all users that have have previously +logged in with LDAP and iterate on them. Each user's sync will start with +the following line that contains the user's username and email, as they +exist in GitLab now: + +```shell +Syncing user John, email@example.com +``` + +If you don't find a particular user's GitLab email in the output, then that +user hasn't logged in with LDAP yet. + +Next, GitLab searches its `identities` table for the existing +link between this user and the configured LDAP provider(s): + +```sql + Identity Load (0.9ms) SELECT "identities".* FROM "identities" WHERE "identities"."user_id" = 20 AND (provider LIKE 'ldap%') LIMIT 1 +``` + +The identity object will have the DN that GitLab will use to look for the user +in LDAP. If the DN isn't found, the email is used instead. We can see that +this user is found in LDAP: + +```shell +Instantiating Gitlab::Auth::Ldap::Person with LDIF: +dn: cn=John Smith,ou=people,dc=example,dc=com +cn: John Smith +mail: email@example.com +memberof: cn=admin_staff,ou=people,dc=example,dc=com +uid: John +``` + +If the user wasn't found in LDAP with either the DN or email, you may see the +following message instead: + +```shell +LDAP search error: No Such Object +``` + +...in which case the user will be blocked: + +```shell + User Update (0.4ms) UPDATE "users" SET "state" = $1, "updated_at" = $2 WHERE "users"."id" = $3 [["state", "ldap_blocked"], ["updated_at", "2019-10-18 15:46:22.902177"], ["id", 20]] +``` + +Once the user is found in LDAP the rest of the output will update the GitLab +database with any changes. + +#### Query a user in LDAP + +This will test that GitLab can reach out to LDAP and read a particular user. +It can expose potential errors connecting to and/or querying LDAP +that may seem to fail silently in the GitLab UI. + +```ruby +Rails.logger.level = Logger::DEBUG + +adapter = Gitlab::Auth::Ldap::Adapter.new('ldapmain') # If `main` is the LDAP provider +Gitlab::Auth::Ldap::Person.find_by_uid('<uid>', adapter) +``` + +### Group memberships **(STARTER ONLY)** + +#### Membership(s) not granted **(STARTER ONLY)** + +Sometimes you may think a particular user should be added to a GitLab group via +LDAP group sync, but for some reason it's not happening. There are several +things to check to debug the situation. + +- Ensure LDAP configuration has a `group_base` specified. + [This configuration][group-sync] is required for group sync to work properly. +- Ensure the correct [LDAP group link is added to the GitLab + group][group-links]. +- Check that the user has an LDAP identity: + 1. Sign in to GitLab as an administrator user. + 1. Navigate to **Admin area -> Users**. + 1. Search for the user + 1. Open the user, by clicking on their name. Do not click 'Edit'. + 1. Navigate to the **Identities** tab. There should be an LDAP identity with + an LDAP DN as the 'Identifier'. If not, this user hasn't logged in with + LDAP yet and must do so first. +- You've waited an hour or [the configured + interval](ldap-ee.md#adjusting-ldap-group-sync-schedule) for the group to + sync. To speed up the process, either go to the GitLab group **Settings -> + Members** and press **Sync now** (sync one group) or [run the group sync rake + task][group-sync-rake] (sync all groups). + +If all of the above looks good, jump in to a little more advanced debugging in +the rails console. + +1. Enter the [rails console](#rails-console). +1. Choose a GitLab group to test with. This group should have an LDAP group link + already configured. +1. [Enable debug logging, find the above GitLab group, and sync it with LDAP](#sync-one-group-starter-only). +1. Look through the output of the sync. See [example log + output](#example-console-output-after-a-group-sync-starter-only) + for how to read the output. +1. If you still aren't able to see why the user isn't being added, [query the + LDAP group directly](#query-a-group-in-ldap-starter-only) to see what members are listed. +1. Is the user's DN or UID in one of the lists from the above output? One of the DNs or + UIDs here should match the 'Identifier' from the LDAP identity checked earlier. If it doesn't, + the user does not appear to be in the LDAP group. + +#### Admin privileges not granted + +When [Administrator sync](ldap-ee.md#administrator-sync) has been configured +but the configured users aren't granted the correct admin privileges, confirm +the following are true: + +- A [`group_base` is also configured](ldap-ee.md#group-sync). +- The configured `admin_group` in the `gitlab.rb` is a CN, rather than a DN or an array. +- This CN falls under the scope of the configured `group_base`. +- The members of the `admin_group` have already logged into GitLab with their LDAP + credentials. GitLab will only grant this admin access to the users whose + accounts are already connected to LDAP. + +If all the above are true and the users are still not getting access, [run a manual +group sync](#sync-all-groups-starter-only) in the rails console and [look through the +output](#example-console-output-after-a-group-sync-starter-only) to see what happens when +GitLab syncs the `admin_group`. + +#### Sync all groups **(STARTER ONLY)** + +NOTE: **NOTE:** +To sync all groups manually when debugging is unnecessary, [use the rake +task][group-sync-rake] instead. + +The output from a manual [group sync][group-sync] can show you what happens +when GitLab syncs its LDAP group memberships against LDAP. + +```ruby +Rails.logger.level = Logger::DEBUG + +LdapAllGroupsSyncWorker.new.perform +``` + +Next, [learn how to read the +output](#example-console-output-after-a-group-sync-starter-only). + +##### Example console output after a group sync **(STARTER ONLY)** + +Like the output from the user sync, the output from the [manual group +sync](#sync-all-groups-starter-only) will also be very verbose. However, it contains lots +of helpful information. + +Indicates the point where syncing actually begins: + +```shell +Started syncing 'ldapmain' provider for 'my_group' group +``` + +The following entry shows an array of all user DNs GitLab sees in the LDAP server. +Note that these are the users for a single LDAP group, not a GitLab group. If +you have multiple LDAP groups linked to this GitLab group, you will see multiple +log entries like this - one for each LDAP group. If you don't see an LDAP user +DN in this log entry, LDAP is not returning the user when we do the lookup. +Verify the user is actually in the LDAP group. + +```shell +Members in 'ldap_group_1' LDAP group: ["uid=john0,ou=people,dc=example,dc=com", +"uid=mary0,ou=people,dc=example,dc=com", "uid=john1,ou=people,dc=example,dc=com", +"uid=mary1,ou=people,dc=example,dc=com", "uid=john2,ou=people,dc=example,dc=com", +"uid=mary2,ou=people,dc=example,dc=com", "uid=john3,ou=people,dc=example,dc=com", +"uid=mary3,ou=people,dc=example,dc=com", "uid=john4,ou=people,dc=example,dc=com", +"uid=mary4,ou=people,dc=example,dc=com"] +``` + +Shortly after each of the above entries, you will see a hash of resolved member +access levels. This hash represents all user DNs GitLab thinks should have +access to this group, and at which access level (role). This hash is additive, +and more DNs may be added, or existing entries modified, based on additional +LDAP group lookups. The very last occurrence of this entry should indicate +exactly which users GitLab believes should be added to the group. + +NOTE: **Note:** +10 is 'Guest', 20 is 'Reporter', 30 is 'Developer', 40 is 'Maintainer' +and 50 is 'Owner'. + +```shell +Resolved 'my_group' group member access: {"uid=john0,ou=people,dc=example,dc=com"=>30, +"uid=mary0,ou=people,dc=example,dc=com"=>30, "uid=john1,ou=people,dc=example,dc=com"=>30, +"uid=mary1,ou=people,dc=example,dc=com"=>30, "uid=john2,ou=people,dc=example,dc=com"=>30, +"uid=mary2,ou=people,dc=example,dc=com"=>30, "uid=john3,ou=people,dc=example,dc=com"=>30, +"uid=mary3,ou=people,dc=example,dc=com"=>30, "uid=john4,ou=people,dc=example,dc=com"=>30, +"uid=mary4,ou=people,dc=example,dc=com"=>30} +``` + +It's not uncommon to see warnings like the following. These indicate that GitLab +would have added the user to a group, but the user could not be found in GitLab. +Usually this is not a cause for concern. + +If you think a particular user should already exist in GitLab, but you're seeing +this entry, it could be due to a mismatched DN stored in GitLab. See +[User DN and/or email have changed](#user-dn-orand-email-have-changed) to update the user's LDAP identity. + +```shell +User with DN `uid=john0,ou=people,dc=example,dc=com` should have access +to 'my_group' group but there is no user in GitLab with that +identity. Membership will be updated once the user signs in for +the first time. +``` + +Finally, the following entry says syncing has finished for this group: + +```shell +Finished syncing all providers for 'my_group' group +``` + +Once all the configured group links have been synchronized, GitLab will look +for any Administrators or External users to sync: + +```shell +Syncing admin users for 'ldapmain' provider +``` + +The output will look similar to what happens with a single group, and then +this line will indicate the sync is finished: + +```shell +Finished syncing admin users for 'ldapmain' provider +``` + +If [admin sync][admin-sync] is not configured, you'll see a message +stating as such: + +```shell +No `admin_group` configured for 'ldapmain' provider. Skipping +``` + +#### Sync one group **(STARTER ONLY)** + +[Syncing all groups](#sync-all-groups-starter-only) can produce a lot of noise in the output, which can be +distracting when you're only interested in troubleshooting the memberships of +a single GitLab group. In that case, here's how you can just sync this group +and see its debug output: + +```ruby +Rails.logger.level = Logger::DEBUG + +# Find the GitLab group. +# If the output is `nil`, the group could not be found. +# If a bunch of group attributes are in the output, your group was found successfully. +group = Group.find_by(name: 'my_gitlab_group') + +# Sync this group against LDAP +EE::Gitlab::Auth::Ldap::Sync::Group.execute_all_providers(group) +``` + +The output will be similar to +[that you'd get from syncing all groups](#example-console-output-after-a-group-sync-starter-only). + +#### Query a group in LDAP **(STARTER ONLY)** + +When you'd like to confirm that GitLab can read a LDAP group and see all its members, +you can run the following: + +```ruby +# Find the adapter and the group itself +adapter = Gitlab::Auth::Ldap::Adapter.new('ldapmain') # If `main` is the LDAP provider +ldap_group = EE::Gitlab::Auth::Ldap::Group.find_by_cn('group_cn_here', adapter) + +# Find the members of the LDAP group +ldap_group.member_dns +ldap_group.member_uids +``` + +### User DN or/and email have changed + +When an LDAP user is created in GitLab, their LDAP DN is stored for later reference. + +If GitLab cannot find a user by their DN, it will fall back +to finding the user by their email. If the lookup is successful, GitLab will +update the stored DN to the new value so both values will now match what's in +LDAP. + +If the email has changed and the DN has not, GitLab will find the user with +the DN and update its own record of the user's email to match the one in LDAP. + +However, if the primary email _and_ the DN change in LDAP, then GitLab will +have no way of identifying the correct LDAP record of the user and, as a +result, the user will be blocked. To rectify this, the user's existing +profile will have to be updated with at least one of the new values (primary +email or DN) so the LDAP record can be found. + +The following script will update the emails for all provided users so they +won't be blocked or unable to access their accounts. + +>**NOTE**: The following script will require that any new accounts with the new +email address are removed first. This is because emails have to be unique in GitLab. + +Go to the [rails console](#rails-console) and then run: + +```ruby +# Each entry will have to include the old username and the new email +emails = { + 'ORIGINAL_USERNAME' => 'NEW_EMAIL_ADDRESS', + ... +} + +emails.each do |username, email| + user = User.find_by_username(username) + user.email = email + user.skip_reconfirmation! + user.save! +end +``` + +You can then [run a UserSync](#sync-all-users-starter-only) **(STARTER ONLY)** to sync the latest DN +for each of these users. + +## Debugging Tools + +### LDAP check + +The [rake task to check LDAP][ldap-check] is a valuable tool +to help determine whether GitLab can successfully establish a connection to +LDAP and can get so far as to even read users. + +If a connection can't be established, it is likely either because of a problem +with your configuration or a firewall blocking the connection. + +- Ensure you don't have a firewall blocking the +connection, and that the LDAP server is accessible to the GitLab host. +- Look for an error message in the rake check output, which may lead to your LDAP configuration to +confirm that the configuration values (specifically `host`, `port`, `bind_dn`, and +`password`) are correct. +- Look for [errors](#connection) in [the logs](#gitlab-logs) to further debug connection failures. + +If GitLab can successfully connect to LDAP but doesn't return any +users, [see what to do when no users are found](#no-users-are-found). + +### GitLab logs + +If a user account is blocked or unblocked due to the LDAP configuration, a +message will be [logged to `application.log`][application-log]. + +If there is an unexpected error during an LDAP lookup (configuration error, +timeout), the login is rejected and a message will be [logged to +`production.log`][production-log]. + +### ldapsearch + +`ldapsearch` is a utility that will allow you to query your LDAP server. You can +use it to test your LDAP settings and ensure that the settings you're using +will get you the results you expect. + +When using `ldapsearch`, be sure to use the same settings you've already +specified in your `gitlab.rb` configuration so you can confirm what happens +when those exact settings are used. + +Running this command on the GitLab host will also help confirm that there's no +obstruction between the GitLab host and LDAP. + +For example, consider the following GitLab configuration: + +```shell +gitlab_rails['ldap_servers'] = YAML.load <<-'EOS' # remember to close this block with 'EOS' below + main: # 'main' is the GitLab 'provider ID' of this LDAP server + label: 'LDAP' + host: '127.0.0.1' + port: 389 + uid: 'uid' + encryption: 'plain' + bind_dn: 'cn=admin,dc=ldap-testing,dc=example,dc=com' + password: 'Password1' + active_directory: true + allow_username_or_email_login: false + block_auto_created_users: false + base: 'dc=ldap-testing,dc=example,dc=com' + user_filter: '' + attributes: + username: ['uid', 'userid', 'sAMAccountName'] + email: ['mail', 'email', 'userPrincipalName'] + name: 'cn' + first_name: 'givenName' + last_name: 'sn' + group_base: 'ou=groups,dc=ldap-testing,dc=example,dc=com' + admin_group: 'gitlab_admin' +EOS +``` + +You would run the following `ldapsearch` to find the `bind_dn` user: + +```shell +ldapsearch -D "cn=admin,dc=ldap-testing,dc=example,dc=com" \ + -w Password1 \ + -p 389 \ + -h 127.0.0.1 \ + -b "dc=ldap-testing,dc=example,dc=com" +``` + +Note that the `bind_dn`, `password`, `port`, `host`, and `base` are all +identical to what's configured in the `gitlab.rb`. + +Please see [the official +`ldapsearch` documentation](https://linux.die.net/man/1/ldapsearch) for more. + +### Rails console + +CAUTION: **CAUTION:** +Please note that it is very easy to create, read, modify, and destroy data on the +rails console, so please be sure to run commands exactly as listed. + +The rails console is a valuable tool to help debug LDAP problems. It allows you to +directly interact with the application by running commands and seeing how GitLab +responds to them. + +Please refer to [this guide](https://docs.gitlab.com/omnibus/maintenance/#starting-a-rails-console-session) +for instructions on how to use the rails console. + +#### Enable debug output + +This will provide debug output that will be useful to see +what GitLab is doing and with what. This value is not persisted, and will only +be enabled for this session in the rails console. + +To enable debug output in the rails console, [enter the rails +console](#rails-console) and run: + +```ruby +Rails.logger.level = Logger::DEBUG +``` + +<!-- LINK REFERENCES --> + +[tail-logs]: https://docs.gitlab.com/omnibus/settings/logs.html#tail-logs-in-a-console-on-the-server +[production-log]: ../logs.md#productionlog +[application-log]: ../logs.md#applicationlog +[reconfigure]: ../restart_gitlab.md#omnibus-gitlab-reconfigure +[restart]: ../restart_gitlab.md#installations-from-source +[ldap-check]: ../raketasks/ldap.md#check +[group-sync-rake]: ../raketasks/ldap.md#run-a-group-sync +[user-filter]: ldap.md#using-an-ldap-filter-to-limit-access-to-your-gitlab-server +[user-sync]: ldap-ee.md#user-sync +[group-sync]: ldap-ee.md#group-sync +[admin-sync]: ldap-ee.md#administrator-sync +[config]: ldap.md#configuration +[group-links]: ldap-ee.md#adding-group-links + +[^1]: In Active Directory, a user is marked as disabled/blocked if the user + account control attribute (`userAccountControl:1.2.840.113556.1.4.803`) + has bit 2 set. See <https://ctogonewild.com/2009/09/03/bitmask-searches-in-ldap/> + for more information. diff --git a/doc/administration/auth/ldap.md b/doc/administration/auth/ldap.md index 067fdfd0018..12a42ec0a1e 100644 --- a/doc/administration/auth/ldap.md +++ b/doc/administration/auth/ldap.md @@ -552,74 +552,4 @@ be mandatory and clients cannot be authenticated with the TLS protocol. ## Troubleshooting -If a user account is blocked or unblocked due to the LDAP configuration, a -message will be logged to `application.log`. - -If there is an unexpected error during an LDAP lookup (configuration error, -timeout), the login is rejected and a message will be logged to -`production.log`. - -### Debug LDAP user filter with ldapsearch - -This example uses `ldapsearch` and assumes you are using ActiveDirectory. The -following query returns the login names of the users that will be allowed to -log in to GitLab if you configure your own user_filter. - -```shell -ldapsearch -H ldaps://$host:$port -D "$bind_dn" -y bind_dn_password.txt -b "$base" "$user_filter" sAMAccountName -``` - -- Variables beginning with a `$` refer to a variable from the LDAP section of - your configuration file. -- Replace `ldaps://` with `ldap://` if you are using the plain authentication method. - Port `389` is the default `ldap://` port and `636` is the default `ldaps://` - port. -- We are assuming the password for the bind_dn user is in bind_dn_password.txt. - -### Invalid credentials when logging in - -- Make sure the user you are binding with has enough permissions to read the user's - tree and traverse it. -- Check that the `user_filter` is not blocking otherwise valid users. -- Run the following check command to make sure that the LDAP settings are - correct and GitLab can see your users: - - ```shell - # For Omnibus installations - sudo gitlab-rake gitlab:ldap:check - - # For installations from source - sudo -u git -H bundle exec rake gitlab:ldap:check RAILS_ENV=production - ``` - -### Connection refused - -If you are getting 'Connection Refused' errors when trying to connect to the -LDAP server please double-check the LDAP `port` and `encryption` settings used by -GitLab. Common combinations are `encryption: 'plain'` and `port: 389`, OR -`encryption: 'simple_tls'` and `port: 636`. - -### Connection times out - -If GitLab cannot reach your LDAP endpoint, you will see a message like this: - -```plaintext -Could not authenticate you from Ldapmain because "Connection timed out - user specified timeout". -``` - -If your configured LDAP provider and/or endpoint is offline or otherwise unreachable by GitLab, no LDAP user will be able to authenticate and log in. GitLab does not cache or store credentials for LDAP users to provide authentication during an LDAP outage. - -Contact your LDAP provider or administrator if you are seeing this error. - -### No file specified as Settingslogic source - -If `sudo gitlab-ctl reconfigure` fails with the following error, or you are seeing it in -the logs, you may have malformed YAML in `/etc/gitlab/gitlab.rb`: - -```plaintext -Errno::ENOENT: No such file or directory - No file specified as Settingslogic source -``` - -This issue is frequently due to the spacing in your YAML file. To fix the problem, -verify the syntax with **spacing** against the -[documentation for the configuration of LDAP](#configuration). +Please see our [administrator guide to troubleshooting LDAP](ldap-troubleshooting.md). diff --git a/doc/administration/raketasks/ldap.md b/doc/administration/raketasks/ldap.md index 1e604d84263..361ab28f6d4 100644 --- a/doc/administration/raketasks/ldap.md +++ b/doc/administration/raketasks/ldap.md @@ -28,7 +28,7 @@ rake gitlab:ldap:check[50] ## Run a Group Sync -> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/14735) in [GitLab Starter](https://about.gitlab.com/pricing/) 12.3. +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/14735) in [GitLab Starter](https://about.gitlab.com/pricing/) 12.2. The following task will run a [group sync](../auth/ldap-ee.md#group-sync) immediately. This is valuable when you'd like to update all configured group memberships against LDAP without diff --git a/doc/administration/troubleshooting/gitlab_rails_cheat_sheet.md b/doc/administration/troubleshooting/gitlab_rails_cheat_sheet.md index ec7b4c20462..15ed436fb34 100644 --- a/doc/administration/troubleshooting/gitlab_rails_cheat_sheet.md +++ b/doc/administration/troubleshooting/gitlab_rails_cheat_sheet.md @@ -536,98 +536,6 @@ group = Group.find_by_path_or_name('group-name') group.project_creation_level=0 ``` -## LDAP - -### LDAP commands in the rails console - -TIP: **TIP:** -Use the rails runner to avoid entering the rails console in the first place. -This is great when only a single command (such as a UserSync or GroupSync) -is needed. - -```ruby -# Get debug output -Rails.logger.level = Logger::DEBUG - -# Run a UserSync (normally performed once a day) -LdapSyncWorker.new.perform - -# Run a GroupSync for all groups (9.3-) -LdapGroupSyncWorker.new.perform - -# Run a GroupSync for all groups (9.3+) -LdapAllGroupsSyncWorker.new.perform - -# Run a GroupSync for a single group (10.6-) -group = Group.find_by(name: 'my_gitlab_group') -EE::Gitlab::LDAP::Sync::Group.execute_all_providers(group) - -# Run a GroupSync for a single group (10.6+) -group = Group.find_by(name: 'my_gitlab_group') -EE::Gitlab::Auth::Ldap::Sync::Group.execute_all_providers(group) - -# Query an LDAP group directly (10.6-) -adapter = Gitlab::LDAP::Adapter.new('ldapmain') # If `main` is the LDAP provider -ldap_group = EE::Gitlab::LDAP::Group.find_by_cn('group_cn_here', adapter) -ldap_group.member_dns -ldap_group.member_uids - -# Query an LDAP group directly (10.6+) -adapter = Gitlab::Auth::Ldap::Adapter.new('ldapmain') # If `main` is the LDAP provider -ldap_group = EE::Gitlab::Auth::Ldap::Group.find_by_cn('group_cn_here', adapter) -ldap_group.member_dns -ldap_group.member_uids - -# Lookup a particular user (10.6+) -# This could expose potential errors connecting to and/or querying LDAP that may seem to -# fail silently in the GitLab UI -adapter = Gitlab::Auth::Ldap::Adapter.new('ldapmain') # If `main` is the LDAP provider -user = Gitlab::Auth::Ldap::Person.find_by_uid('<username>',adapter) - -# Query the LDAP server directly (10.6+) -## For an example, see https://gitlab.com/gitlab-org/gitlab/blob/master/ee/lib/ee/gitlab/auth/ldap/adapter.rb -adapter = Gitlab::Auth::Ldap::Adapter.new('ldapmain') -options = { - # the :base is required - # use adapter.config.base for the base or .group_base for the group_base - base: adapter.config.group_base, - - # :filter is optional - # 'cn' looks for all "cn"s under :base - # '*' is the search string - here, it's a wildcard - filter: Net::LDAP::Filter.eq('cn', '*'), - - # :attributes is optional - # the attributes we want to get returned - attributes: %w(dn cn memberuid member submember uniquemember memberof) -} -adapter.ldap_search(options) -``` - -### Update user accounts when the `dn` and email change - -The following will require that any accounts with the new email address are removed. -Emails have to be unique in GitLab. This is expected to work but unverified as of yet: - -```ruby -# Here's an example with a couple users. -# Each entry will have to include the old username and the new email -emails = { - 'ORIGINAL_USERNAME' => 'NEW_EMAIL_ADDRESS', - ... -} - -emails.each do |username, email| - user = User.find_by_username(username) - user.email = email - user.skip_reconfirmation! - user.save! -end - -# Run the UserSync to update the above users' data -LdapSyncWorker.new.perform -``` - ## Routes ### Remove redirecting routes diff --git a/doc/development/application_limits.md b/doc/development/application_limits.md index f50730634b7..3592bb29f85 100644 --- a/doc/development/application_limits.md +++ b/doc/development/application_limits.md @@ -20,40 +20,45 @@ limits](https://about.gitlab.com/handbook/product/#introducing-application-limit In the `plan_limits` table, you have to create a new column and insert the limit values. It's recommended to create separate migration script files. -1. Add new column to the `plan_limits` table with non-null default value 0, eg: - - ```ruby - add_column(:plan_limits, :project_hooks, :integer, default: 0, null: false) - ``` - - NOTE: **Note:** Plan limits entries set to `0` mean that limits are not - enabled. - -1. Insert plan limits values into the database using - `create_or_update_plan_limit` migration helper, eg: - - ```ruby - def up - return unless Gitlab.com? - - create_or_update_plan_limit('project_hooks', 'free', 100) - create_or_update_plan_limit('project_hooks', 'bronze', 100) - create_or_update_plan_limit('project_hooks', 'silver', 100) - create_or_update_plan_limit('project_hooks', 'gold', 100) - end - - def down - return unless Gitlab.com? - - create_or_update_plan_limit('project_hooks', 'free', 0) - create_or_update_plan_limit('project_hooks', 'bronze', 0) - create_or_update_plan_limit('project_hooks', 'silver', 0) - create_or_update_plan_limit('project_hooks', 'gold', 0) - end - ``` - -NOTE: **Note:** Some plans exist only on GitLab.com. You can check if the -migration is running on GitLab.com with `Gitlab.com?`. +1. Add new column to the `plan_limits` table with non-null default value + that represents desired limit, eg: + + ```ruby + add_column(:plan_limits, :project_hooks, :integer, default: 100, null: false) + ``` + + NOTE: **Note:** Plan limits entries set to `0` mean that limits are not + enabled. You should use this setting only in special and documented circumstances. + +1. (Optionally) Create the database migration that fine-tunes each level with + a desired limit using `create_or_update_plan_limit` migration helper, eg: + + ```ruby + class InsertProjectHooksPlanLimits < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def up + create_or_update_plan_limit('project_hooks', 'default', 0) + create_or_update_plan_limit('project_hooks', 'free', 10) + create_or_update_plan_limit('project_hooks', 'bronze', 20) + create_or_update_plan_limit('project_hooks', 'silver', 30) + create_or_update_plan_limit('project_hooks', 'gold', 100) + end + + def down + create_or_update_plan_limit('project_hooks', 'default', 0) + create_or_update_plan_limit('project_hooks', 'free', 0) + create_or_update_plan_limit('project_hooks', 'bronze', 0) + create_or_update_plan_limit('project_hooks', 'silver', 0) + create_or_update_plan_limit('project_hooks', 'gold', 0) + end + end + ``` + +NOTE: **Note:** Some plans exist only on GitLab.com. This will be no-op +for plans that do not exist. ### Plan limits validation diff --git a/doc/development/fe_guide/vuex.md b/doc/development/fe_guide/vuex.md index 675f30feba6..fd8ba0297c5 100644 --- a/doc/development/fe_guide/vuex.md +++ b/doc/development/fe_guide/vuex.md @@ -246,21 +246,96 @@ From [vuex mutations docs](https://vuex.vuejs.org/guide/mutations.html): export const ADD_USER = 'ADD_USER'; ``` -### How to include the store in your application +### Initializing a store's state -The store should be included in the main component of your application: +It's common for a Vuex store to need some initial state before its `action`s can +be used. Often this includes data like API endpoints, documentation URLs, or +IDs. + +To set this initial state, pass it as a parameter to your store's creation +function when mounting your Vue component: ```javascript - // app.vue - import store from './store'; // it will include the index.js file +// in the Vue app's initialization script (e.g. mount_show.js) - export default { - name: 'application', - store, - ... - }; +import Vue from 'vue'; +import createStore from './stores'; +import AwesomeVueApp from './components/awesome_vue_app.vue' + +export default () => { + const el = document.getElementById('js-awesome-vue-app'); + + return new Vue({ + el, + store: createStore(el.dataset), + render: h => h(AwesomeVueApp) + }); +}; ``` +The store function, in turn, can pass this data along to the state's creation +function: + +```javascript +// in store/index.js + +import * as actions from './actions'; +import mutations from './mutations'; +import createState from './state'; + +export default initialState => ({ + actions, + mutations, + state: createState(initialState), +}); +``` + +And the state function can accept this initial data as a parameter and bake it +into the `state` object it returns: + +```javascript +// in store/state.js + +export default ({ + projectId, + documentationPath, + anOptionalProperty = true +}) => ({ + projectId, + documentationPath, + anOptionalProperty, + + // other state properties here +}); +``` + +#### Why not just ...spread the initial state? + +The astute reader will see an opportunity to cut out a few lines of code from +the example above: + +```javascript +// Don't do this! + +export default initialState => ({ + ...initialState, + + // other state properties here +}); +``` + +We've made the conscious decision to avoid this pattern to aid in the +discoverability and searchability of our frontend codebase. The reasoning for +this is described in [this +discussion](https://gitlab.com/gitlab-org/frontend/rfcs/-/issues/56#note_302514865): + +> Consider a `someStateKey` is being used in the store state. You _may_ not be +> able to grep for it directly if it was provided only by `el.dataset`. Instead, +> you'd have to grep for `some_state_key`, since it could have come from a rails +> template. The reverse is also true: if you're looking at a rails template, you +> might wonder what uses `some_state_key`, but you'd _have_ to grep for +> `someStateKey` + ### Communicating with the Store ```javascript diff --git a/doc/user/project/releases/index.md b/doc/user/project/releases/index.md index 279ed35b20b..53559ef1bb8 100644 --- a/doc/user/project/releases/index.md +++ b/doc/user/project/releases/index.md @@ -332,6 +332,19 @@ background job. If a past `released_at` is used, no Evidence is collected for the Release. +## GitLab Releaser + +> [Introduced](https://gitlab.com/gitlab-org/gitlab-releaser/-/merge_requests/6) in GitLab 12.10. + +GitLab Releaser is a CLI tool for managing GitLab Releases from the command line or from +GitLab CI/CD's configuration file, `.gitlab-ci.yml`. + +With it, you can create, update, modify, and delete Releases right through the +terminal. + +Read the [GitLab Releaser documentation](https://gitlab.com/gitlab-org/gitlab-releaser/-/tree/master/docs/index.md) +for details. + <!-- ## Troubleshooting Include any troubleshooting steps that you can foresee. If you know beforehand what issues diff --git a/lib/gitlab/database/migration_helpers.rb b/lib/gitlab/database/migration_helpers.rb index 82a84508959..64eef530663 100644 --- a/lib/gitlab/database/migration_helpers.rb +++ b/lib/gitlab/database/migration_helpers.rb @@ -1121,11 +1121,14 @@ into similar problems in the future (e.g. when new tables are created). end def create_or_update_plan_limit(limit_name, plan_name, limit_value) + limit_name_quoted = quote_column_name(limit_name) + plan_name_quoted = quote(plan_name) + limit_value_quoted = quote(limit_value) + execute <<~SQL - INSERT INTO plan_limits (plan_id, #{quote_column_name(limit_name)}) - VALUES - ((SELECT id FROM plans WHERE name = #{quote(plan_name)} LIMIT 1), #{quote(limit_value)}) - ON CONFLICT (plan_id) DO UPDATE SET #{quote_column_name(limit_name)} = EXCLUDED.#{quote_column_name(limit_name)}; + INSERT INTO plan_limits (plan_id, #{limit_name_quoted}) + SELECT id, #{limit_value_quoted} FROM plans WHERE name = #{plan_name_quoted} LIMIT 1 + ON CONFLICT (plan_id) DO UPDATE SET #{limit_name_quoted} = EXCLUDED.#{limit_name_quoted}; SQL end diff --git a/lib/gitlab/x509/commit.rb b/lib/gitlab/x509/commit.rb index 4b35c0ef7d2..91951a3e505 100644 --- a/lib/gitlab/x509/commit.rb +++ b/lib/gitlab/x509/commit.rb @@ -31,175 +31,23 @@ module Gitlab end end - def verified_signature - strong_memoize(:verified_signature) { verified_signature? } - end - - def cert - strong_memoize(:cert) do - signer_certificate(p7) if valid_signature? - end - end - - def cert_store - strong_memoize(:cert_store) do - store = OpenSSL::X509::Store.new - store.set_default_paths - # valid_signing_time? checks the time attributes already - # this flag is required, otherwise expired certificates would become - # unverified when notAfter within certificate attribute is reached - store.flags = OpenSSL::X509::V_FLAG_NO_CHECK_TIME - store - end - end - - def p7 - strong_memoize(:p7) do - pkcs7_text = signature_text.sub('-----BEGIN SIGNED MESSAGE-----', '-----BEGIN PKCS7-----') - pkcs7_text = pkcs7_text.sub('-----END SIGNED MESSAGE-----', '-----END PKCS7-----') - - OpenSSL::PKCS7.new(pkcs7_text) - rescue - nil - end - end - - def valid_signing_time? - # rfc 5280 - 4.1.2.5 Validity - # check if signed_time is within the time range (notBefore/notAfter) - # non-rfc - git specific check: signed_time >= commit_time - p7.signers[0].signed_time.between?(cert.not_before, cert.not_after) && - p7.signers[0].signed_time >= @commit.created_at - end - - def valid_signature? - p7.verify([], cert_store, signed_text, OpenSSL::PKCS7::NOVERIFY) - rescue - nil - end - - def verified_signature? - # verify has multiple options but only a boolean return value - # so first verify without certificate chain - if valid_signature? - if valid_signing_time? - # verify with system certificate chain - p7.verify([], cert_store, signed_text) - else - false - end - else - nil - end - rescue - nil - end - - def signer_certificate(p7) - p7.certificates.each do |cert| - next if cert.serial != p7.signers[0].serial - - return cert - end - end - - def certificate_crl - extension = get_certificate_extension('crlDistributionPoints') - crl_url = nil - - extension.each_line do |line| - break if crl_url - - line.split('URI:').each do |item| - item.strip - - if item.start_with?("http") - crl_url = item.strip - break - end - end - end - - crl_url - end - - def get_certificate_extension(extension) - cert.extensions.each do |ext| - if ext.oid == extension - return ext.value - end - end - end - - def issuer_subject_key_identifier - get_certificate_extension('authorityKeyIdentifier').gsub("keyid:", "").delete!("\n") - end - - def certificate_subject_key_identifier - get_certificate_extension('subjectKeyIdentifier') - end - - def certificate_issuer - cert.issuer.to_s(OpenSSL::X509::Name::RFC2253) - end - - def certificate_subject - cert.subject.to_s(OpenSSL::X509::Name::RFC2253) - end - - def certificate_email - get_certificate_extension('subjectAltName').split('email:')[1] - end - - def issuer_attributes - return if verified_signature.nil? - - { - subject_key_identifier: issuer_subject_key_identifier, - subject: certificate_issuer, - crl_url: certificate_crl - } - end - - def certificate_attributes - return if verified_signature.nil? - - issuer = X509Issuer.safe_create!(issuer_attributes) - - { - subject_key_identifier: certificate_subject_key_identifier, - subject: certificate_subject, - email: certificate_email, - serial_number: cert.serial, - x509_issuer_id: issuer.id - } - end - def attributes - return if verified_signature.nil? + return if @commit.sha.nil? || @commit.project.nil? - certificate = X509Certificate.safe_create!(certificate_attributes) + signature = X509::Signature.new(signature_text, signed_text, @commit.committer_email, @commit.created_at) + + return if signature.verified_signature.nil? || signature.x509_certificate.nil? { commit_sha: @commit.sha, project: @commit.project, - x509_certificate_id: certificate.id, - verification_status: verification_status(certificate) + x509_certificate_id: signature.x509_certificate.id, + verification_status: signature.verification_status } end - def verification_status(certificate) - return :unverified if certificate.revoked? - - if verified_signature && certificate_email == @commit.committer_email - :verified - else - :unverified - end - end - def create_cached_signature! - return if verified_signature.nil? + return if attributes.nil? return X509CommitSignature.new(attributes) if Gitlab::Database.read_only? diff --git a/lib/gitlab/x509/signature.rb b/lib/gitlab/x509/signature.rb new file mode 100644 index 00000000000..ed248e29211 --- /dev/null +++ b/lib/gitlab/x509/signature.rb @@ -0,0 +1,198 @@ +# frozen_string_literal: true +require 'openssl' +require 'digest' + +module Gitlab + module X509 + class Signature + include Gitlab::Utils::StrongMemoize + + attr_reader :signature_text, :signed_text, :created_at + + def initialize(signature_text, signed_text, email, created_at) + @signature_text = signature_text + @signed_text = signed_text + @email = email + @created_at = created_at + end + + def x509_certificate + return if certificate_attributes.nil? + + X509Certificate.safe_create!(certificate_attributes) unless verified_signature.nil? + end + + def verified_signature + strong_memoize(:verified_signature) { verified_signature? } + end + + def verification_status + return :unverified if x509_certificate.nil? || x509_certificate.revoked? + + if verified_signature && certificate_email == @email + :verified + else + :unverified + end + end + + private + + def cert + strong_memoize(:cert) do + signer_certificate(p7) if valid_signature? + end + end + + def cert_store + strong_memoize(:cert_store) do + store = OpenSSL::X509::Store.new + store.set_default_paths + # valid_signing_time? checks the time attributes already + # this flag is required, otherwise expired certificates would become + # unverified when notAfter within certificate attribute is reached + store.flags = OpenSSL::X509::V_FLAG_NO_CHECK_TIME + store + end + end + + def p7 + strong_memoize(:p7) do + pkcs7_text = signature_text.sub('-----BEGIN SIGNED MESSAGE-----', '-----BEGIN PKCS7-----') + pkcs7_text = pkcs7_text.sub('-----END SIGNED MESSAGE-----', '-----END PKCS7-----') + + OpenSSL::PKCS7.new(pkcs7_text) + rescue + nil + end + end + + def valid_signing_time? + # rfc 5280 - 4.1.2.5 Validity + # check if signed_time is within the time range (notBefore/notAfter) + # non-rfc - git specific check: signed_time >= commit_time + p7.signers[0].signed_time.between?(cert.not_before, cert.not_after) && + p7.signers[0].signed_time >= created_at + end + + def valid_signature? + p7.verify([], cert_store, signed_text, OpenSSL::PKCS7::NOVERIFY) + rescue + nil + end + + def verified_signature? + # verify has multiple options but only a boolean return value + # so first verify without certificate chain + if valid_signature? + if valid_signing_time? + # verify with system certificate chain + p7.verify([], cert_store, signed_text) + else + false + end + else + nil + end + rescue + nil + end + + def signer_certificate(p7) + p7.certificates.each do |cert| + next if cert.serial != p7.signers[0].serial + + return cert + end + end + + def certificate_crl + extension = get_certificate_extension('crlDistributionPoints') + return if extension.nil? + + crl_url = nil + + extension.each_line do |line| + break if crl_url + + line.split('URI:').each do |item| + item.strip + + if item.start_with?("http") + crl_url = item.strip + break + end + end + end + + crl_url + end + + def get_certificate_extension(extension) + ext = cert.extensions.detect { |ext| ext.oid == extension } + ext&.value + end + + def issuer_subject_key_identifier + key_identifier = get_certificate_extension('authorityKeyIdentifier') + return if key_identifier.nil? + + key_identifier.gsub("keyid:", "").delete!("\n") + end + + def certificate_subject_key_identifier + key_identifier = get_certificate_extension('subjectKeyIdentifier') + return if key_identifier.nil? + + key_identifier + end + + def certificate_issuer + cert.issuer.to_s(OpenSSL::X509::Name::RFC2253) + end + + def certificate_subject + cert.subject.to_s(OpenSSL::X509::Name::RFC2253) + end + + def certificate_email + email = nil + + get_certificate_extension('subjectAltName').split(',').each do |item| + if item.strip.start_with?("email") + email = item.split('email:')[1] + break + end + end + + return if email.nil? + + email + end + + def x509_issuer + return if verified_signature.nil? || issuer_subject_key_identifier.nil? || certificate_crl.nil? + + attributes = { + subject_key_identifier: issuer_subject_key_identifier, + subject: certificate_issuer, + crl_url: certificate_crl + } + + X509Issuer.safe_create!(attributes) unless verified_signature.nil? + end + + def certificate_attributes + return if verified_signature.nil? || certificate_subject_key_identifier.nil? || x509_issuer.nil? + + { + subject_key_identifier: certificate_subject_key_identifier, + subject: certificate_subject, + email: certificate_email, + serial_number: cert.serial.to_i, + x509_issuer_id: x509_issuer.id + } + end + end + end +end diff --git a/spec/frontend/blob/blob_file_dropzone_spec.js b/spec/frontend/blob/blob_file_dropzone_spec.js index 4e9a05418df..cbd36abd4ff 100644 --- a/spec/frontend/blob/blob_file_dropzone_spec.js +++ b/spec/frontend/blob/blob_file_dropzone_spec.js @@ -5,10 +5,6 @@ describe('BlobFileDropzone', () => { preloadFixtures('blob/show.html'); let dropzone; let replaceFileButton; - const jQueryMock = { - enable: jest.fn(), - disable: jest.fn(), - }; beforeEach(() => { loadFixtures('blob/show.html'); @@ -18,7 +14,6 @@ describe('BlobFileDropzone', () => { dropzone = $('.js-upload-blob-form .dropzone').get(0).dropzone; dropzone.processQueue = jest.fn(); replaceFileButton = $('#submit-all'); - $.fn.extend(jQueryMock); }); describe('submit button', () => { @@ -43,7 +38,7 @@ describe('BlobFileDropzone', () => { replaceFileButton.click(); expect(window.alert).not.toHaveBeenCalled(); - expect(jQueryMock.enable).toHaveBeenCalled(); + expect(replaceFileButton.is(':disabled')).toEqual(true); expect(dropzone.processQueue).toHaveBeenCalled(); }); }); diff --git a/spec/frontend/boards/board_new_issue_spec.js b/spec/frontend/boards/board_new_issue_spec.js index 4eb7f0c131e..94afc8a2b45 100644 --- a/spec/frontend/boards/board_new_issue_spec.js +++ b/spec/frontend/boards/board_new_issue_spec.js @@ -1,6 +1,5 @@ /* global List */ -import $ from 'jquery'; import Vue from 'vue'; import MockAdapter from 'axios-mock-adapter'; import axios from '~/lib/utils/axios_utils'; @@ -15,9 +14,6 @@ describe('Issue boards new issue form', () => { let list; let mock; let newIssueMock; - const jQueryMock = { - enable: jest.fn(), - }; const promiseReturn = { data: { iid: 100, @@ -53,8 +49,6 @@ describe('Issue boards new issue form', () => { }, }).$mount(document.querySelector('.test-container')); - $.fn.extend(jQueryMock); - return Vue.nextTick(); }); @@ -118,7 +112,7 @@ describe('Issue boards new issue form', () => { return Vue.nextTick() .then(submitIssue) .then(() => { - expect(jQueryMock.enable).toHaveBeenCalled(); + expect(vm.$el.querySelector('.btn-success').disabled).toBe(false); }); }); diff --git a/spec/frontend/diffs/components/diff_file_header_spec.js b/spec/frontend/diffs/components/diff_file_header_spec.js index 8e6a5576015..e0b7e0bc0f3 100644 --- a/spec/frontend/diffs/components/diff_file_header_spec.js +++ b/spec/frontend/diffs/components/diff_file_header_spec.js @@ -61,7 +61,6 @@ describe('DiffFileHeader component', () => { const findTitleLink = () => wrapper.find({ ref: 'titleWrapper' }); const findExpandButton = () => wrapper.find({ ref: 'expandDiffToFullFileButton' }); const findFileActions = () => wrapper.find('.file-actions'); - const findActiveHeader = () => wrapper.find('.is-active'); const findModeChangedLine = () => wrapper.find({ ref: 'fileMode' }); const findLfsLabel = () => wrapper.find('.label-lfs'); const findToggleDiscussionsButton = () => wrapper.find({ ref: 'toggleDiscussionsButton' }); @@ -144,11 +143,6 @@ describe('DiffFileHeader component', () => { expect(wrapper.find(ClipboardButton).exists()).toBe(true); }); - it('contains a active header class if this is the active file header', () => { - createComponent({ isActive: true }); - expect(findActiveHeader().exists()).toBe(true); - }); - describe('for submodule', () => { const submoduleDiffFile = { ...diffFile, diff --git a/spec/frontend/helpers/jquery.js b/spec/frontend/helpers/jquery.js index 6421a592c0c..4af5f904394 100644 --- a/spec/frontend/helpers/jquery.js +++ b/spec/frontend/helpers/jquery.js @@ -1,6 +1,18 @@ import $ from 'jquery'; +// Expose jQuery so specs using jQuery plugins can be imported nicely. +// Here is an issue to explore better alternatives: +// https://gitlab.com/gitlab-org/gitlab/issues/12448 global.$ = $; global.jQuery = $; +// Fail tests for unmocked requests +$.ajax = () => { + const err = new Error( + 'Unexpected unmocked jQuery.ajax() call! Make sure to mock jQuery.ajax() in tests.', + ); + global.fail(err); + throw err; +}; + export default $; diff --git a/spec/frontend/mocks/node/jquery.js b/spec/frontend/mocks/node/jquery.js deleted file mode 100644 index 5c82f65406e..00000000000 --- a/spec/frontend/mocks/node/jquery.js +++ /dev/null @@ -1,15 +0,0 @@ -/* eslint-disable import/no-commonjs */ - -const $ = jest.requireActual('jquery'); - -// Fail tests for unmocked requests -$.ajax = () => { - const err = new Error( - 'Unexpected unmocked jQuery.ajax() call! Make sure to mock jQuery.ajax() in tests.', - ); - global.fail(err); - throw err; -}; - -// jquery is not an ES6 module -module.exports = $; diff --git a/spec/frontend/notes/components/notes_app_spec.js b/spec/frontend/notes/components/notes_app_spec.js index 2d0cca18647..60e866542a6 100644 --- a/spec/frontend/notes/components/notes_app_spec.js +++ b/spec/frontend/notes/components/notes_app_spec.js @@ -1,4 +1,4 @@ -import $ from 'helpers/jquery'; +import $ from 'jquery'; import AxiosMockAdapter from 'axios-mock-adapter'; import Vue from 'vue'; import { mount } from '@vue/test-utils'; diff --git a/spec/frontend/releases/stores/modules/detail/actions_spec.js b/spec/frontend/releases/stores/modules/detail/actions_spec.js index 88346083f5a..70f7432c65d 100644 --- a/spec/frontend/releases/stores/modules/detail/actions_spec.js +++ b/spec/frontend/releases/stores/modules/detail/actions_spec.js @@ -24,7 +24,14 @@ describe('Release detail actions', () => { let error; beforeEach(() => { - state = createState(); + state = createState({ + projectId: '18', + tagName: 'v1.3', + releasesPagePath: 'path/to/releases/page', + markdownDocsPath: 'path/to/markdown/docs', + markdownPreviewPath: 'path/to/markdown/preview', + updateReleaseApiDocsPath: 'path/to/api/docs', + }); release = cloneDeep(originalRelease); mock = new MockAdapter(axios); gon.api_version = 'v4'; @@ -36,16 +43,6 @@ describe('Release detail actions', () => { mock.restore(); }); - describe('setInitialState', () => { - it(`commits ${types.SET_INITIAL_STATE} with the provided object`, () => { - const initialState = {}; - - return testAction(actions.setInitialState, initialState, state, [ - { type: types.SET_INITIAL_STATE, payload: initialState }, - ]); - }); - }); - describe('requestRelease', () => { it(`commits ${types.REQUEST_RELEASE}`, () => testAction(actions.requestRelease, undefined, state, [{ type: types.REQUEST_RELEASE }])); diff --git a/spec/frontend/releases/stores/modules/detail/mutations_spec.js b/spec/frontend/releases/stores/modules/detail/mutations_spec.js index 81b2dde75ab..d49c8854ca2 100644 --- a/spec/frontend/releases/stores/modules/detail/mutations_spec.js +++ b/spec/frontend/releases/stores/modules/detail/mutations_spec.js @@ -5,115 +5,106 @@ * is resolved */ -import state from '~/releases/stores/modules/detail/state'; +import createState from '~/releases/stores/modules/detail/state'; import mutations from '~/releases/stores/modules/detail/mutations'; import * as types from '~/releases/stores/modules/detail/mutation_types'; import { release } from '../../../mock_data'; describe('Release detail mutations', () => { - let stateClone; + let state; let releaseClone; beforeEach(() => { - stateClone = state(); - releaseClone = JSON.parse(JSON.stringify(release)); - }); - - describe(types.SET_INITIAL_STATE, () => { - it('populates the state with initial values', () => { - const initialState = { - projectId: '18', - tagName: 'v1.3', - releasesPagePath: 'path/to/releases/page', - markdownDocsPath: 'path/to/markdown/docs', - markdownPreviewPath: 'path/to/markdown/preview', - }; - - mutations[types.SET_INITIAL_STATE](stateClone, initialState); - - expect(stateClone).toEqual(expect.objectContaining(initialState)); + state = createState({ + projectId: '18', + tagName: 'v1.3', + releasesPagePath: 'path/to/releases/page', + markdownDocsPath: 'path/to/markdown/docs', + markdownPreviewPath: 'path/to/markdown/preview', + updateReleaseApiDocsPath: 'path/to/api/docs', }); + releaseClone = JSON.parse(JSON.stringify(release)); }); describe(types.REQUEST_RELEASE, () => { it('set state.isFetchingRelease to true', () => { - mutations[types.REQUEST_RELEASE](stateClone); + mutations[types.REQUEST_RELEASE](state); - expect(stateClone.isFetchingRelease).toEqual(true); + expect(state.isFetchingRelease).toEqual(true); }); }); describe(types.RECEIVE_RELEASE_SUCCESS, () => { it('handles a successful response from the server', () => { - mutations[types.RECEIVE_RELEASE_SUCCESS](stateClone, releaseClone); + mutations[types.RECEIVE_RELEASE_SUCCESS](state, releaseClone); - expect(stateClone.fetchError).toEqual(undefined); + expect(state.fetchError).toEqual(undefined); - expect(stateClone.isFetchingRelease).toEqual(false); + expect(state.isFetchingRelease).toEqual(false); - expect(stateClone.release).toEqual(releaseClone); + expect(state.release).toEqual(releaseClone); }); }); describe(types.RECEIVE_RELEASE_ERROR, () => { it('handles an unsuccessful response from the server', () => { const error = { message: 'An error occurred!' }; - mutations[types.RECEIVE_RELEASE_ERROR](stateClone, error); + mutations[types.RECEIVE_RELEASE_ERROR](state, error); - expect(stateClone.isFetchingRelease).toEqual(false); + expect(state.isFetchingRelease).toEqual(false); - expect(stateClone.release).toBeUndefined(); + expect(state.release).toBeUndefined(); - expect(stateClone.fetchError).toEqual(error); + expect(state.fetchError).toEqual(error); }); }); describe(types.UPDATE_RELEASE_TITLE, () => { it("updates the release's title", () => { - stateClone.release = releaseClone; + state.release = releaseClone; const newTitle = 'The new release title'; - mutations[types.UPDATE_RELEASE_TITLE](stateClone, newTitle); + mutations[types.UPDATE_RELEASE_TITLE](state, newTitle); - expect(stateClone.release.name).toEqual(newTitle); + expect(state.release.name).toEqual(newTitle); }); }); describe(types.UPDATE_RELEASE_NOTES, () => { it("updates the release's notes", () => { - stateClone.release = releaseClone; + state.release = releaseClone; const newNotes = 'The new release notes'; - mutations[types.UPDATE_RELEASE_NOTES](stateClone, newNotes); + mutations[types.UPDATE_RELEASE_NOTES](state, newNotes); - expect(stateClone.release.description).toEqual(newNotes); + expect(state.release.description).toEqual(newNotes); }); }); describe(types.REQUEST_UPDATE_RELEASE, () => { it('set state.isUpdatingRelease to true', () => { - mutations[types.REQUEST_UPDATE_RELEASE](stateClone); + mutations[types.REQUEST_UPDATE_RELEASE](state); - expect(stateClone.isUpdatingRelease).toEqual(true); + expect(state.isUpdatingRelease).toEqual(true); }); }); describe(types.RECEIVE_UPDATE_RELEASE_SUCCESS, () => { it('handles a successful response from the server', () => { - mutations[types.RECEIVE_UPDATE_RELEASE_SUCCESS](stateClone, releaseClone); + mutations[types.RECEIVE_UPDATE_RELEASE_SUCCESS](state, releaseClone); - expect(stateClone.updateError).toEqual(undefined); + expect(state.updateError).toEqual(undefined); - expect(stateClone.isUpdatingRelease).toEqual(false); + expect(state.isUpdatingRelease).toEqual(false); }); }); describe(types.RECEIVE_UPDATE_RELEASE_ERROR, () => { it('handles an unsuccessful response from the server', () => { const error = { message: 'An error occurred!' }; - mutations[types.RECEIVE_UPDATE_RELEASE_ERROR](stateClone, error); + mutations[types.RECEIVE_UPDATE_RELEASE_ERROR](state, error); - expect(stateClone.isUpdatingRelease).toEqual(false); + expect(state.isUpdatingRelease).toEqual(false); - expect(stateClone.updateError).toEqual(error); + expect(state.updateError).toEqual(error); }); }); }); diff --git a/spec/frontend/test_setup.js b/spec/frontend/test_setup.js index 203781bb6fc..fff76f158dd 100644 --- a/spec/frontend/test_setup.js +++ b/spec/frontend/test_setup.js @@ -1,6 +1,5 @@ import Vue from 'vue'; import * as jqueryMatchers from 'custom-jquery-matchers'; -import $ from 'jquery'; import { config as testUtilsConfig } from '@vue/test-utils'; import Translate from '~/vue_shared/translate'; import { initializeTestTimeout } from './helpers/timeout'; @@ -9,11 +8,9 @@ import { setupManualMocks } from './mocks/mocks_helper'; import customMatchers from './matchers'; import './helpers/dom_shims'; - -// Expose jQuery so specs using jQuery plugins can be imported nicely. -// Here is an issue to explore better alternatives: -// https://gitlab.com/gitlab-org/gitlab/issues/12448 -window.jQuery = $; +import './helpers/jquery'; +import '~/commons/jquery'; +import '~/commons/bootstrap'; process.on('unhandledRejection', global.promiseRejectionHandler); diff --git a/spec/frontend/vue_shared/components/file_row_spec.js b/spec/frontend/vue_shared/components/file_row_spec.js index 75d1ce9cc5b..b3ced84ddb5 100644 --- a/spec/frontend/vue_shared/components/file_row_spec.js +++ b/spec/frontend/vue_shared/components/file_row_spec.js @@ -72,19 +72,6 @@ describe('File row component', () => { }); }); - it('is marked as viewed if clicked', () => { - createComponent({ - file: { - ...file(), - type: 'blob', - fileHash: '#123456789', - }, - level: 0, - viewedFiles: ['#123456789'], - }); - expect(wrapper.classes()).toContain('is-viewed'); - }); - it('indents row based on level', () => { createComponent({ file: file('t4'), diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb index 1fd6157ce43..9ac2660908c 100644 --- a/spec/lib/gitlab/database/migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migration_helpers_spec.rb @@ -1542,16 +1542,54 @@ describe Gitlab::Database::MigrationHelpers do end describe '#create_or_update_plan_limit' do - it 'creates or updates plan limits' do + class self::Plan < ActiveRecord::Base + self.table_name = 'plans' + end + + class self::PlanLimits < ActiveRecord::Base + self.table_name = 'plan_limits' + end + + it 'properly escapes names' do expect(model).to receive(:execute).with <<~SQL INSERT INTO plan_limits (plan_id, "project_hooks") - VALUES - ((SELECT id FROM plans WHERE name = 'free' LIMIT 1), '10') + SELECT id, '10' FROM plans WHERE name = 'free' LIMIT 1 ON CONFLICT (plan_id) DO UPDATE SET "project_hooks" = EXCLUDED."project_hooks"; SQL model.create_or_update_plan_limit('project_hooks', 'free', 10) end + + context 'when plan does not exist' do + it 'does not create any plan limits' do + expect { model.create_or_update_plan_limit('project_hooks', 'plan_name', 10) } + .not_to change { self.class::PlanLimits.count } + end + end + + context 'when plan does exist' do + let!(:plan) { self.class::Plan.create!(name: 'plan_name') } + + context 'when limit does not exist' do + it 'inserts a new plan limits' do + expect { model.create_or_update_plan_limit('project_hooks', 'plan_name', 10) } + .to change { self.class::PlanLimits.count }.by(1) + + expect(self.class::PlanLimits.pluck(:project_hooks)).to contain_exactly(10) + end + end + + context 'when limit does exist' do + let!(:plan_limit) { self.class::PlanLimits.create!(plan_id: plan.id) } + + it 'updates an existing plan limits' do + expect { model.create_or_update_plan_limit('project_hooks', 'plan_name', 999) } + .not_to change { self.class::PlanLimits.count } + + expect(plan_limit.reload.project_hooks).to eq(999) + end + end + end end describe '#with_lock_retries' do diff --git a/spec/lib/gitlab/x509/commit_spec.rb b/spec/lib/gitlab/x509/commit_spec.rb index 07d7eba6b9a..ac93609b467 100644 --- a/spec/lib/gitlab/x509/commit_spec.rb +++ b/spec/lib/gitlab/x509/commit_spec.rb @@ -5,252 +5,30 @@ describe Gitlab::X509::Commit do describe '#signature' do let(:signature) { described_class.new(commit).signature } - let(:user1_certificate_attributes) do - { - subject_key_identifier: X509Helpers::User1.certificate_subject_key_identifier, - subject: X509Helpers::User1.certificate_subject, - email: X509Helpers::User1.certificate_email, - serial_number: X509Helpers::User1.certificate_serial - } - end - - let(:user1_issuer_attributes) do - { - subject_key_identifier: X509Helpers::User1.issuer_subject_key_identifier, - subject: X509Helpers::User1.certificate_issuer, - crl_url: X509Helpers::User1.certificate_crl - } - end + context 'returns the cached signature' do + let(:commit_sha) { '189a6c924013fc3fe40d6f1ec1dc20214183bc97' } + let(:project) { create(:project, :public, :repository) } + let(:commit) { create(:commit, project: project, sha: commit_sha) } - shared_examples 'returns the cached signature on second call' do - it 'returns the cached signature on second call' do - x509_commit = described_class.new(commit) + it 'on second call' do + allow_any_instance_of(described_class).to receive(:new).and_call_original + expect_any_instance_of(described_class).to receive(:create_cached_signature!).and_call_original - expect(x509_commit).to receive(:create_cached_signature).and_call_original signature # consecutive call - expect(x509_commit).not_to receive(:create_cached_signature).and_call_original + expect(described_class).not_to receive(:create_cached_signature!).and_call_original signature end end - let!(:project) { create :project, :repository, path: X509Helpers::User1.path } - let!(:commit_sha) { X509Helpers::User1.commit } - context 'unsigned commit' do + let!(:project) { create :project, :repository, path: X509Helpers::User1.path } + let!(:commit_sha) { X509Helpers::User1.commit } let!(:commit) { create :commit, project: project, sha: commit_sha } it 'returns nil' do - expect(described_class.new(commit).signature).to be_nil - end - end - - context 'valid signature from known user' do - let!(:commit) { create :commit, project: project, sha: commit_sha, created_at: Time.utc(2019, 1, 1, 20, 15, 0), committer_email: X509Helpers::User1.emails.first } - - let!(:user) { create(:user, email: X509Helpers::User1.emails.first) } - - before do - allow(Gitlab::Git::Commit).to receive(:extract_signature_lazily) - .with(Gitlab::Git::Repository, commit_sha) - .and_return( - [ - X509Helpers::User1.signed_commit_signature, - X509Helpers::User1.signed_commit_base_data - ] - ) - end - - it 'returns an unverified signature' do - expect(signature).to have_attributes( - commit_sha: commit_sha, - project: project, - verification_status: 'unverified' - ) - expect(signature.x509_certificate).to have_attributes(user1_certificate_attributes) - expect(signature.x509_certificate.x509_issuer).to have_attributes(user1_issuer_attributes) - expect(signature.persisted?).to be_truthy - end - end - - context 'verified signature from known user' do - let!(:commit) { create :commit, project: project, sha: commit_sha, created_at: Time.utc(2019, 1, 1, 20, 15, 0), committer_email: X509Helpers::User1.emails.first } - - let!(:user) { create(:user, email: X509Helpers::User1.emails.first) } - - before do - allow(Gitlab::Git::Commit).to receive(:extract_signature_lazily) - .with(Gitlab::Git::Repository, commit_sha) - .and_return( - [ - X509Helpers::User1.signed_commit_signature, - X509Helpers::User1.signed_commit_base_data - ] - ) - end - - context 'with trusted certificate store' do - before do - store = OpenSSL::X509::Store.new - certificate = OpenSSL::X509::Certificate.new X509Helpers::User1.trust_cert - store.add_cert(certificate) - allow(OpenSSL::X509::Store).to receive(:new) - .and_return( - store - ) - end - - it 'returns a verified signature' do - expect(signature).to have_attributes( - commit_sha: commit_sha, - project: project, - verification_status: 'verified' - ) - expect(signature.x509_certificate).to have_attributes(user1_certificate_attributes) - expect(signature.x509_certificate.x509_issuer).to have_attributes(user1_issuer_attributes) - expect(signature.persisted?).to be_truthy - end - - context 'revoked certificate' do - let(:x509_issuer) { create(:x509_issuer, user1_issuer_attributes) } - let!(:x509_certificate) { create(:x509_certificate, user1_certificate_attributes.merge(x509_issuer_id: x509_issuer.id, certificate_status: :revoked)) } - - it 'returns an unverified signature' do - expect(signature).to have_attributes( - commit_sha: commit_sha, - project: project, - verification_status: 'unverified' - ) - expect(signature.x509_certificate).to have_attributes(user1_certificate_attributes) - expect(signature.x509_certificate.x509_issuer).to have_attributes(user1_issuer_attributes) - expect(signature.persisted?).to be_truthy - end - end - end - - context 'without trusted certificate within store' do - before do - store = OpenSSL::X509::Store.new - allow(OpenSSL::X509::Store).to receive(:new) - .and_return( - store - ) - end - - it 'returns an unverified signature' do - expect(signature).to have_attributes( - commit_sha: commit_sha, - project: project, - verification_status: 'unverified' - ) - expect(signature.x509_certificate).to have_attributes(user1_certificate_attributes) - expect(signature.x509_certificate.x509_issuer).to have_attributes(user1_issuer_attributes) - expect(signature.persisted?).to be_truthy - end - end - end - - context 'unverified signature from unknown user' do - let!(:commit) { create :commit, project: project, sha: commit_sha, created_at: Time.utc(2019, 1, 1, 20, 15, 0), committer_email: X509Helpers::User1.emails.first } - - before do - allow(Gitlab::Git::Commit).to receive(:extract_signature_lazily) - .with(Gitlab::Git::Repository, commit_sha) - .and_return( - [ - X509Helpers::User1.signed_commit_signature, - X509Helpers::User1.signed_commit_base_data - ] - ) - end - - it 'returns an unverified signature' do - expect(signature).to have_attributes( - commit_sha: commit_sha, - project: project, - verification_status: 'unverified' - ) - expect(signature.x509_certificate).to have_attributes(user1_certificate_attributes) - expect(signature.x509_certificate.x509_issuer).to have_attributes(user1_issuer_attributes) - expect(signature.persisted?).to be_truthy - end - end - - context 'invalid signature' do - let!(:commit) { create :commit, project: project, sha: commit_sha, committer_email: X509Helpers::User1.emails.first } - - let!(:user) { create(:user, email: X509Helpers::User1.emails.first) } - - before do - allow(Gitlab::Git::Commit).to receive(:extract_signature_lazily) - .with(Gitlab::Git::Repository, commit_sha) - .and_return( - [ - # Corrupt the key - X509Helpers::User1.signed_commit_signature.tr('A', 'B'), - X509Helpers::User1.signed_commit_base_data - ] - ) - end - - it 'returns nil' do - expect(described_class.new(commit).signature).to be_nil - end - end - - context 'invalid commit message' do - let!(:commit) { create :commit, project: project, sha: commit_sha, committer_email: X509Helpers::User1.emails.first } - - let!(:user) { create(:user, email: X509Helpers::User1.emails.first) } - - before do - allow(Gitlab::Git::Commit).to receive(:extract_signature_lazily) - .with(Gitlab::Git::Repository, commit_sha) - .and_return( - [ - X509Helpers::User1.signed_commit_signature, - # Corrupt the commit message - 'x' - ] - ) - end - - it 'returns nil' do - expect(described_class.new(commit).signature).to be_nil - end - end - - context 'certificate_crl' do - let!(:commit) { create :commit, project: project, sha: commit_sha, created_at: Time.utc(2019, 1, 1, 20, 15, 0), committer_email: X509Helpers::User1.emails.first } - let(:signed_commit) { described_class.new(commit) } - - describe 'valid crlDistributionPoints' do - before do - allow(signed_commit).to receive(:get_certificate_extension).and_call_original - - allow(signed_commit).to receive(:get_certificate_extension) - .with('crlDistributionPoints') - .and_return("\nFull Name:\n URI:http://ch.siemens.com/pki?ZZZZZZA2.crl\n URI:ldap://cl.siemens.net/CN=ZZZZZZA2,L=PKI?certificateRevocationList\n URI:ldap://cl.siemens.com/CN=ZZZZZZA2,o=Trustcenter?certificateRevocationList\n") - end - - it 'returns an unverified signature' do - expect(signed_commit.signature.x509_certificate.x509_issuer).to have_attributes(user1_issuer_attributes) - end - end - - describe 'valid crlDistributionPoints providing multiple http URIs' do - before do - allow(signed_commit).to receive(:get_certificate_extension).and_call_original - - allow(signed_commit).to receive(:get_certificate_extension) - .with('crlDistributionPoints') - .and_return("\nFull Name:\n URI:http://cdp1.pca.dfn.de/dfn-ca-global-g2/pub/crl/cacrl.crl\n\nFull Name:\n URI:http://cdp2.pca.dfn.de/dfn-ca-global-g2/pub/crl/cacrl.crl\n") - end - - it 'extracts the first URI' do - expect(signed_commit.signature.x509_certificate.x509_issuer.crl_url).to eq("http://cdp1.pca.dfn.de/dfn-ca-global-g2/pub/crl/cacrl.crl") - end + expect(signature).to be_nil end end end diff --git a/spec/lib/gitlab/x509/signature_spec.rb b/spec/lib/gitlab/x509/signature_spec.rb new file mode 100644 index 00000000000..6c585acd5cd --- /dev/null +++ b/spec/lib/gitlab/x509/signature_spec.rb @@ -0,0 +1,232 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::X509::Signature do + let(:issuer_attributes) do + { + subject_key_identifier: X509Helpers::User1.issuer_subject_key_identifier, + subject: X509Helpers::User1.certificate_issuer, + crl_url: X509Helpers::User1.certificate_crl + } + end + + context 'commit signature' do + let(:certificate_attributes) do + { + subject_key_identifier: X509Helpers::User1.certificate_subject_key_identifier, + subject: X509Helpers::User1.certificate_subject, + email: X509Helpers::User1.certificate_email, + serial_number: X509Helpers::User1.certificate_serial + } + end + + context 'verified signature' do + context 'with trusted certificate store' do + before do + store = OpenSSL::X509::Store.new + certificate = OpenSSL::X509::Certificate.new(X509Helpers::User1.trust_cert) + store.add_cert(certificate) + allow(OpenSSL::X509::Store).to receive(:new).and_return(store) + end + + it 'returns a verified signature if email does match' do + signature = described_class.new( + X509Helpers::User1.signed_commit_signature, + X509Helpers::User1.signed_commit_base_data, + X509Helpers::User1.certificate_email, + X509Helpers::User1.signed_commit_time + ) + + expect(signature.x509_certificate).to have_attributes(certificate_attributes) + expect(signature.x509_certificate.x509_issuer).to have_attributes(issuer_attributes) + expect(signature.verified_signature).to be_truthy + expect(signature.verification_status).to eq(:verified) + end + + it 'returns an unverified signature if email does not match' do + signature = described_class.new( + X509Helpers::User1.signed_commit_signature, + X509Helpers::User1.signed_commit_base_data, + "gitlab@example.com", + X509Helpers::User1.signed_commit_time + ) + + expect(signature.x509_certificate).to have_attributes(certificate_attributes) + expect(signature.x509_certificate.x509_issuer).to have_attributes(issuer_attributes) + expect(signature.verified_signature).to be_truthy + expect(signature.verification_status).to eq(:unverified) + end + + it 'returns an unverified signature if email does match and time is wrong' do + signature = described_class.new( + X509Helpers::User1.signed_commit_signature, + X509Helpers::User1.signed_commit_base_data, + X509Helpers::User1.certificate_email, + Time.new(2020, 2, 22) + ) + + expect(signature.x509_certificate).to have_attributes(certificate_attributes) + expect(signature.x509_certificate.x509_issuer).to have_attributes(issuer_attributes) + expect(signature.verified_signature).to be_falsey + expect(signature.verification_status).to eq(:unverified) + end + + it 'returns an unverified signature if certificate is revoked' do + signature = described_class.new( + X509Helpers::User1.signed_commit_signature, + X509Helpers::User1.signed_commit_base_data, + X509Helpers::User1.certificate_email, + X509Helpers::User1.signed_commit_time + ) + + expect(signature.verification_status).to eq(:verified) + + signature.x509_certificate.revoked! + + expect(signature.verification_status).to eq(:unverified) + end + end + + context 'without trusted certificate within store' do + before do + store = OpenSSL::X509::Store.new + allow(OpenSSL::X509::Store).to receive(:new) + .and_return( + store + ) + end + + it 'returns an unverified signature' do + signature = described_class.new( + X509Helpers::User1.signed_commit_signature, + X509Helpers::User1.signed_commit_base_data, + X509Helpers::User1.certificate_email, + X509Helpers::User1.signed_commit_time + ) + + expect(signature.x509_certificate).to have_attributes(certificate_attributes) + expect(signature.x509_certificate.x509_issuer).to have_attributes(issuer_attributes) + expect(signature.verified_signature).to be_falsey + expect(signature.verification_status).to eq(:unverified) + end + end + end + + context 'invalid signature' do + it 'returns nil' do + signature = described_class.new( + X509Helpers::User1.signed_commit_signature.tr('A', 'B'), + X509Helpers::User1.signed_commit_base_data, + X509Helpers::User1.certificate_email, + X509Helpers::User1.signed_commit_time + ) + expect(signature.x509_certificate).to be_nil + expect(signature.verified_signature).to be_falsey + expect(signature.verification_status).to eq(:unverified) + end + end + + context 'invalid commit message' do + it 'returns nil' do + signature = described_class.new( + X509Helpers::User1.signed_commit_signature, + 'x', + X509Helpers::User1.certificate_email, + X509Helpers::User1.signed_commit_time + ) + expect(signature.x509_certificate).to be_nil + expect(signature.verified_signature).to be_falsey + expect(signature.verification_status).to eq(:unverified) + end + end + end + + context 'certificate_crl' do + describe 'valid crlDistributionPoints' do + before do + allow_any_instance_of(Gitlab::X509::Signature).to receive(:get_certificate_extension).and_call_original + + allow_any_instance_of(Gitlab::X509::Signature).to receive(:get_certificate_extension) + .with('crlDistributionPoints') + .and_return("\nFull Name:\n URI:http://ch.siemens.com/pki?ZZZZZZA2.crl\n URI:ldap://cl.siemens.net/CN=ZZZZZZA2,L=PKI?certificateRevocationList\n URI:ldap://cl.siemens.com/CN=ZZZZZZA2,o=Trustcenter?certificateRevocationList\n") + end + + it 'creates an issuer' do + signature = described_class.new( + X509Helpers::User1.signed_commit_signature, + X509Helpers::User1.signed_commit_base_data, + X509Helpers::User1.certificate_email, + X509Helpers::User1.signed_commit_time + ) + + expect(signature.x509_certificate.x509_issuer).to have_attributes(issuer_attributes) + end + end + + describe 'valid crlDistributionPoints providing multiple http URIs' do + before do + allow_any_instance_of(Gitlab::X509::Signature).to receive(:get_certificate_extension).and_call_original + + allow_any_instance_of(Gitlab::X509::Signature).to receive(:get_certificate_extension) + .with('crlDistributionPoints') + .and_return("\nFull Name:\n URI:http://cdp1.pca.dfn.de/dfn-ca-global-g2/pub/crl/cacrl.crl\n\nFull Name:\n URI:http://cdp2.pca.dfn.de/dfn-ca-global-g2/pub/crl/cacrl.crl\n") + end + + it 'extracts the first URI' do + signature = described_class.new( + X509Helpers::User1.signed_commit_signature, + X509Helpers::User1.signed_commit_base_data, + X509Helpers::User1.certificate_email, + X509Helpers::User1.signed_commit_time + ) + + expect(signature.x509_certificate.x509_issuer.crl_url).to eq("http://cdp1.pca.dfn.de/dfn-ca-global-g2/pub/crl/cacrl.crl") + end + end + end + + context 'email' do + describe 'subjectAltName with email, othername' do + before do + allow_any_instance_of(Gitlab::X509::Signature).to receive(:get_certificate_extension).and_call_original + + allow_any_instance_of(Gitlab::X509::Signature).to receive(:get_certificate_extension) + .with('subjectAltName') + .and_return("email:gitlab@example.com, othername:<unsupported>") + end + + it 'extracts email' do + signature = described_class.new( + X509Helpers::User1.signed_commit_signature, + X509Helpers::User1.signed_commit_base_data, + 'gitlab@example.com', + X509Helpers::User1.signed_commit_time + ) + + expect(signature.x509_certificate.email).to eq("gitlab@example.com") + end + end + + describe 'subjectAltName with othername, email' do + before do + allow_any_instance_of(Gitlab::X509::Signature).to receive(:get_certificate_extension).and_call_original + + allow_any_instance_of(Gitlab::X509::Signature).to receive(:get_certificate_extension) + .with('subjectAltName') + .and_return("othername:<unsupported>, email:gitlab@example.com") + end + + it 'extracts email' do + signature = described_class.new( + X509Helpers::User1.signed_commit_signature, + X509Helpers::User1.signed_commit_base_data, + 'gitlab@example.com', + X509Helpers::User1.signed_commit_time + ) + + expect(signature.x509_certificate.email).to eq("gitlab@example.com") + end + end + end +end diff --git a/spec/requests/api/pipeline_schedules_spec.rb b/spec/requests/api/pipeline_schedules_spec.rb index 05abdf76be9..14b292db045 100644 --- a/spec/requests/api/pipeline_schedules_spec.rb +++ b/spec/requests/api/pipeline_schedules_spec.rb @@ -46,7 +46,7 @@ describe API::PipelineSchedules do get api("/projects/#{project.id}/pipeline_schedules", developer) end.count - create_pipeline_schedules(10) + create_pipeline_schedules(5) expect do get api("/projects/#{project.id}/pipeline_schedules", developer) diff --git a/spec/support/helpers/x509_helpers.rb b/spec/support/helpers/x509_helpers.rb index f72b518134c..9ea997bf5f4 100644 --- a/spec/support/helpers/x509_helpers.rb +++ b/spec/support/helpers/x509_helpers.rb @@ -169,6 +169,10 @@ module X509Helpers SIGNEDDATA end + def signed_commit_time + Time.at(1561027326) + end + def certificate_crl 'http://ch.siemens.com/pki?ZZZZZZA2.crl' end |