Commit caac06f0 authored by Imre Farkas's avatar Imre Farkas

Merge branch '33907-smartcard-flex-single-san-email-entries' into 'master'

Allow to use Smartcard certificates with SAN extensions that only defines one email entry to login without matching URI

See merge request gitlab-org/gitlab!20052
parents 10ffefb9 f1a5ecb7
......@@ -51,10 +51,14 @@ This is an experimental feature. Smartcard authentication against local database
change or be removed completely in future releases.
To use a smartcard with an X.509 certificate to authenticate against a local
database with GitLab, at least one of the `subjectAltName` (SAN) extensions
need to define the user identity (`email`) within the GitLab instance (`URI`).
`URI`: needs to match `Gitlab.config.host.gitlab`.
database with GitLab, in:
- GitLab 12.4 and later, at least one of the `subjectAltName` (SAN) extensions
need to define the user identity (`email`) within the GitLab instance (`URI`).
`URI`: needs to match `Gitlab.config.host.gitlab`.
- From [GitLab 12.5](https://gitlab.com/gitlab-org/gitlab/issues/33907),
if your certificate contains only **one** SAN email entry, you don't need to
add or modify it to match the `email` with the `URI`.
For example:
......
---
title: Allow to login with Smartcard certificates using SAN extensions that only defines
one global email identity
merge_request: 20052
author:
type: changed
......@@ -27,7 +27,14 @@ module Gitlab
end
def email_identity
alternate_emails.find { |name| gitlab_host?(name[URI_TAG]) }&.fetch(EMAIL_TAG, nil)
email_entry =
if alternate_emails.size == 1
alternate_emails.first
else
alternate_emails.find { |name| gitlab_host?(name[URI_TAG]) }
end
email_entry&.fetch(EMAIL_TAG, nil)
end
def alternate_emails
......
......@@ -5,7 +5,7 @@ require 'spec_helper'
describe Gitlab::Auth::Smartcard::SANExtension do
let(:fqdn) { 'gitlab.example.com' }
let(:extension_factory) { OpenSSL::X509::ExtensionFactory.new(nil, cert) }
let(:san_extension) { described_class.new(cert, fqdn)}
let(:san_extension) { described_class.new(cert, fqdn) }
let(:cert) do
key = OpenSSL::PKey::RSA.new 2048
......@@ -53,9 +53,9 @@ describe Gitlab::Auth::Smartcard::SANExtension do
it {
is_expected.to match([{
described_class::EMAIL_TAG => email,
described_class::URI_TAG => uri
}])
described_class::EMAIL_TAG => email,
described_class::URI_TAG => uri
}])
}
end
......@@ -69,42 +69,51 @@ describe Gitlab::Auth::Smartcard::SANExtension do
end
describe '#email_identity' do
let(:san_email) { 'newemail@some.domain' }
let(:san_uri) { "https://#{fqdn}" }
let(:san_single_email) { 'singleEntryEmail@some.domain' }
before do
allow(Gitlab.config.gitlab).to receive(:host).and_return(fqdn)
add_san_entry "email:#{san_single_email}"
end
subject { san_extension.email_identity }
describe 'alternate name email for GitLab defined in the certificate' do
it { is_expected.to eq san_single_email }
context 'multiple email identity SAN entries' do
let(:san_email) { 'newemail@some.domain' }
let(:san_uri) { 'not.yourdomain.com' }
before do
add_san_entry "email:#{san_email},URI:#{san_uri}"
end
it { is_expected.to eq san_email }
describe 'alternate name email for GitLab defined in the certificate' do
let(:san_uri) { "https://#{fqdn}" }
describe 'inappropriate URI format' do
let(:san_uri) { 'an invalid uri' }
it { is_expected.to eq san_email }
context 'inappropriate URI format' do
let(:san_uri) { 'an invalid uri' }
it { is_expected.to be_nil }
end
end
context 'no alternate name defined to use with GitLab' do
it { is_expected.to be_nil }
end
end
describe 'no alternate name defined to use with GitLab' do
it { is_expected.to be_nil }
end
context 'when the host is partially matched to the URI' do
let(:uri) { "https://#{fqdn}.anotherdomain.com" }
let(:identity) { 'user@email.com' }
context 'when the host is partially matched to the URI' do
let(:forged_uri) { "https://#{fqdn}.anotherdomain.com" }
let(:forged_identity) { 'hacker@email.com' }
before do
add_san_entry "email:#{identity},URI:#{uri}"
end
before do
add_san_entry "email:#{forged_identity},URI:#{forged_uri}"
it { is_expected.to be_nil }
end
it { is_expected.to be_nil }
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment