kotovalexarian-likes-gitlab/gitlab-org--gitlab-foss
1
0
Fork 0
Code Releases Activity

Don't hash user ID in OIDC subject claim

Browse source
This commit is contained in:
Markus Koller 2018-06-13 22:32:21 +02:00
parent f63e234b57
commit 904b6dd083
No known key found for this signature in database
GPG key ID: A2B74A05A7A2B7B7
7 changed files with 90 additions and 71 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
Gemfile
View file

@ -35,7 +35,7 @@ gem 'faraday', '~> 0.12'
# Authentication libraries
gem 'devise', '~> 4.4'
gem 'doorkeeper', '~> 4.3'
gem 'doorkeeper-openid_connect', '~> 1.3'
gem 'doorkeeper-openid_connect', '~> 1.5'
gem 'omniauth', '~> 1.8'
gem 'omniauth-auth0', '~> 2.0.0'
gem 'omniauth-azure-oauth2', '~> 0.0.9'

12
Gemfile.lock
View file

@ -171,7 +171,7 @@ GEM
unf (>= 0.0.5, < 1.0.0)
doorkeeper (4.3.2)
railties (>= 4.2)
doorkeeper-openid_connect (1.4.0)
doorkeeper-openid_connect (1.5.0)
doorkeeper (~> 4.3)
json-jwt (~> 1.6)
dropzonejs-rails (0.7.2)
@ -427,12 +427,10 @@ GEM
oauth (~> 0.5, >= 0.5.0)
jquery-atwho-rails (1.3.2)
json (1.8.6)
json-jwt (1.9.2)
json-jwt (1.9.4)
activesupport
aes_key_wrap
bindata
securecompare
url_safe_base64
json-schema (2.8.0)
addressable (>= 2.4)
jwt (1.5.6)
@ -512,7 +510,7 @@ GEM
net-ldap (0.16.0)
net-ssh (5.0.1)
netrc (0.11.0)
nokogiri (1.8.2)
nokogiri (1.8.3)
mini_portile2 (~> 2.3.0)
nokogumbo (1.5.0)
nokogiri
@ -827,7 +825,6 @@ GEM
scss_lint (0.56.0)
rake (>= 0.9, < 13)
sass (~> 3.5.3)
securecompare (1.0.0)
seed-fu (2.3.7)
activerecord (>= 3.1)
activesupport (>= 3.1)
@ -939,7 +936,6 @@ GEM
equalizer (~> 0.0.9)
parser (>= 2.3.1.2, < 2.6)
procto (~> 0.0.2)
url_safe_base64 (0.2.2)
validates_hostname (1.0.6)
activerecord (>= 3.0)
activesupport (>= 3.0)
@ -1013,7 +1009,7 @@ DEPENDENCIES
devise-two-factor (~> 3.0.0)
diffy (~> 3.1.0)
doorkeeper (~> 4.3)
doorkeeper-openid_connect (~> 1.3)
doorkeeper-openid_connect (~> 1.5)
dropzonejs-rails (~> 0.7.1)
ed25519 (~> 1.2)
email_reply_trimmer (~> 0.1)

10
Gemfile.rails5.lock
View file

@ -174,7 +174,7 @@ GEM
unf (>= 0.0.5, < 1.0.0)
doorkeeper (4.3.2)
railties (>= 4.2)
doorkeeper-openid_connect (1.4.0)
doorkeeper-openid_connect (1.5.0)
doorkeeper (~> 4.3)
json-jwt (~> 1.6)
dropzonejs-rails (0.7.2)
@ -430,12 +430,10 @@ GEM
oauth (~> 0.5, >= 0.5.0)
jquery-atwho-rails (1.3.2)
json (1.8.6)
json-jwt (1.9.2)
json-jwt (1.9.4)
activesupport
aes_key_wrap
bindata
securecompare
url_safe_base64
json-schema (2.8.0)
addressable (>= 2.4)
jwt (1.5.6)
@ -836,7 +834,6 @@ GEM
scss_lint (0.56.0)
rake (>= 0.9, < 13)
sass (~> 3.5.3)
securecompare (1.0.0)
seed-fu (2.3.7)
activerecord (>= 3.1)
activesupport (>= 3.1)
@ -946,7 +943,6 @@ GEM
equalizer (~> 0.0.9)
parser (>= 2.3.1.2, < 2.6)
procto (~> 0.0.2)
url_safe_base64 (0.2.2)
validates_hostname (1.0.6)
activerecord (>= 3.0)
activesupport (>= 3.0)
@ -1023,7 +1019,7 @@ DEPENDENCIES
devise-two-factor (~> 3.0.0)
diffy (~> 3.1.0)
doorkeeper (~> 4.3)
doorkeeper-openid_connect (~> 1.3)
doorkeeper-openid_connect (~> 1.5)
dropzonejs-rails (~> 0.7.1)
ed25519 (~> 1.2)
email_reply_trimmer (~> 0.1)

5
changelogs/unreleased/feature-oidc-subject-claim.yml Normal file
View file

@ -0,0 +1,5 @@
---
title: Don't hash user ID in OIDC subject claim
merge_request: 19784
author: Markus Koller
type: changed

9
config/initializers/doorkeeper_openid_connect.rb
View file

@ -18,12 +18,17 @@ Doorkeeper::OpenidConnect.configure do
end
subject do |user|
# hash the user's ID with the Rails secret_key_base to avoid revealing it
Digest::SHA256.hexdigest "#{user.id}-#{Rails.application.secrets.secret_key_base}"
user.id
end
claims do
with_options scope: :openid do |o|
o.claim(:sub_legacy, response: [:id_token, :user_info]) do |user|
# provide the previously hashed 'sub' claim to allow third-party apps
# to migrate to the new unhashed value
Digest::SHA256.hexdigest "#{user.id}-#{Rails.application.secrets.secret_key_base}"
end
o.claim(:name) { |user| user.name }
o.claim(:nickname) { |user| user.username }
o.claim(:email) { |user| user.public_email }

13
doc/integration/openid_connect_provider.md
View file

@ -5,11 +5,11 @@ to sign in to other services.
## Introduction to OpenID Connect
[OpenID Connect] \(OIC) is a simple identity layer on top of the
[OpenID Connect] \(OIDC) is a simple identity layer on top of the
OAuth 2.0 protocol. It allows clients to verify the identity of the end-user
based on the authentication performed by GitLab, as well as to obtain
basic profile information about the end-user in an interoperable and
REST-like manner. OIC performs many of the same tasks as OpenID 2.0,
REST-like manner. OIDC performs many of the same tasks as OpenID 2.0,
but does so in a way that is API-friendly, and usable by native and
mobile applications.
@ -23,14 +23,17 @@ are supported.
## Enabling OpenID Connect for OAuth applications
Refer to the [OAuth guide] for basic information on how to set up OAuth
applications in GitLab. To enable OIC for an application, all you have to do
applications in GitLab. To enable OIDC for an application, all you have to do
is select the `openid` scope in the application settings.
## Shared information
Currently the following user information is shared with clients:
| Claim | Type | Description |
|:-----------------|:----------|:------------|
| `sub` | `string` | An opaque token that uniquely identifies the user
| `sub` | `string` | The ID of the user
| `sub_legacy` | `string` | An opaque token that uniquely identifies the user<br><br>**Deprecation notice:** this token isn't stable because it's tied to the Rails secret key base, and is provided only for migration to the new stable `sub` value available from GitLab 11.1
| `auth_time` | `integer` | The timestamp for the user's last authentication
| `name` | `string` | The user's full name
| `nickname` | `string` | The user's GitLab username
@ -41,6 +44,8 @@ Currently the following user information is shared with clients:
| `picture` | `string` | URL for the user's GitLab avatar
| `groups` | `array` | Names of the groups the user is a member of
Only the `sub` and `sub_legacy` claims are included in the ID token, all other claims are available from the `/oauth/userinfo` endpoint used by OIDC clients.
[OpenID Connect]: http://openid.net/connect/ "OpenID Connect website"
[doorkeeper-openid_connect]: https://github.com/doorkeeper-gem/doorkeeper-openid_connect "Doorkeeper::OpenidConnect website"
[OAuth guide]: oauth_provider.md "GitLab as OAuth2 authentication service provider"

110
spec/requests/openid_connect_spec.rb
View file

@ -1,11 +1,49 @@
require 'spec_helper'
describe 'OpenID Connect requests' do
let(:user) { create :user }
let(:user) do
create(
:user,
name: 'Alice',
username: 'alice',
email: 'private@example.com',
emails: [public_email],
public_email: public_email.email,
website_url: 'https://example.com',
avatar: fixture_file_upload('spec/fixtures/dk.png')
)
end
let(:public_email) { build :email, email: 'public@example.com' }
let(:access_grant) { create :oauth_access_grant, application: application, resource_owner_id: user.id }
let(:access_token) { create :oauth_access_token, application: application, resource_owner_id: user.id }
def request_access_token
let(:hashed_subject) do
Digest::SHA256.hexdigest("#{user.id}-#{Rails.application.secrets.secret_key_base}")
end
let(:id_token_claims) do
{
'sub' => user.id.to_s,
'sub_legacy' => hashed_subject
}
end
let(:user_info_claims) do
{
'name' => 'Alice',
'nickname' => 'alice',
'email' => 'public@example.com',
'email_verified' => true,
'website' => 'https://example.com',
'profile' => 'http://localhost/alice',
'picture' => "http://localhost/uploads/-/system/user/avatar/#{user.id}/dk.png",
'groups' => kind_of(Array)
}
end
def request_access_token!
login_as user
post '/oauth/token',
@ -16,26 +54,22 @@ describe 'OpenID Connect requests' do
client_secret: application.secret
end
def request_user_info
def request_user_info!
get '/oauth/userinfo', nil, 'Authorization' => "Bearer #{access_token.token}"
end
def hashed_subject
Digest::SHA256.hexdigest("#{user.id}-#{Rails.application.secrets.secret_key_base}")
end
context 'Application without OpenID scope' do
let(:application) { create :oauth_application, scopes: 'api' }
it 'token response does not include an ID token' do
request_access_token
request_access_token!
expect(json_response).to include 'access_token'
expect(json_response).not_to include 'id_token'
end
it 'userinfo response is unauthorized' do
request_user_info
request_user_info!
expect(response).to have_gitlab_http_status 403
expect(response.body).to be_blank
@ -46,28 +80,12 @@ describe 'OpenID Connect requests' do
let(:application) { create :oauth_application, scopes: 'openid' }
it 'token response includes an ID token' do
request_access_token
request_access_token!
expect(json_response).to include 'id_token'
end
context 'UserInfo payload' do
let(:user) do
create(
:user,
name: 'Alice',
username: 'alice',
emails: [private_email, public_email],
email: private_email.email,
public_email: public_email.email,
website_url: 'https://example.com',
avatar: fixture_file_upload('spec/fixtures/dk.png')
)
end
let!(:public_email) { build :email, email: 'public@example.com' }
let!(:private_email) { build :email, email: 'private@example.com' }
let!(:group1) { create :group }
let!(:group2) { create :group }
let!(:group3) { create :group, parent: group2 }
@ -76,49 +94,43 @@ describe 'OpenID Connect requests' do
before do
group1.add_user(user, GroupMember::OWNER)
group3.add_user(user, Gitlab::Access::DEVELOPER)
request_user_info!
end
it 'includes all user information and group memberships' do
request_user_info
expect(json_response).to match(a_hash_including({
'sub' => hashed_subject,
'name' => 'Alice',
'nickname' => 'alice',
'email' => 'public@example.com',
'email_verified' => true,
'website' => 'https://example.com',
'profile' => 'http://localhost/alice',
'picture' => "http://localhost/uploads/-/system/user/avatar/#{user.id}/dk.png",
'groups' => anything
}))
expect(json_response).to match(id_token_claims.merge(user_info_claims))
expected_groups = [group1.full_path, group3.full_path]
expected_groups << group4.full_path if Group.supports_nested_groups?
expect(json_response['groups']).to match_array(expected_groups)
end
it 'does not include any unknown claims' do
expect(json_response.keys).to eq %w[sub sub_legacy] + user_info_claims.keys
end
end
context 'ID token payload' do
before do
request_access_token
request_access_token!
@payload = JSON::JWT.decode(json_response['id_token'], :skip_verification)
end
it 'includes the subject claims' do
expect(@payload).to match(a_hash_including(id_token_claims))
end
it 'includes the Gitlab root URL' do
expect(@payload['iss']).to eq Gitlab.config.gitlab.url
end
it 'includes the hashed user ID' do
expect(@payload['sub']).to eq hashed_subject
end
it 'includes the time of the last authentication', :clean_gitlab_redis_shared_state do
expect(@payload['auth_time']).to eq user.current_sign_in_at.to_i
end
it 'does not include any unknown properties' do
expect(@payload.keys).to eq %w[iss sub aud exp iat auth_time]
expect(@payload.keys).to eq %w[iss sub aud exp iat auth_time sub_legacy]
end
end
@ -134,10 +146,10 @@ describe 'OpenID Connect requests' do
context 'when user is blocked' do
it 'returns authentication error' do
access_grant
user.block
user.block!
expect do
request_access_token
request_access_token!
end.to raise_error UncaughtThrowError
end
end
@ -145,10 +157,10 @@ describe 'OpenID Connect requests' do
context 'when user is ldap_blocked' do
it 'returns authentication error' do
access_grant
user.ldap_block
user.ldap_block!
expect do
request_access_token
request_access_token!
end.to raise_error UncaughtThrowError
end
end