diff options
author | Douwe Maan <douwe@gitlab.com> | 2018-06-28 17:59:36 +0300 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2018-06-28 17:59:36 +0300 |
commit | 6be703dd59cd35fb94ee4496a2d52af696d8cd70 (patch) | |
tree | c7c7721420fea3f07a4ff0bee48c57b5e703b017 /spec | |
parent | 50a213112ee422bea6673cf15a8c4829ac8700ee (diff) | |
parent | 904b6dd0834868ec260f40077623463926114373 (diff) |
Merge branch 'feature/oidc-subject-claim' into 'master'
Don't hash user ID in OIDC subject claim
Closes #47791
See merge request gitlab-org/gitlab-ce!19784
Diffstat (limited to 'spec')
-rw-r--r-- | spec/requests/openid_connect_spec.rb | 110 |
1 files changed, 61 insertions, 49 deletions
diff --git a/spec/requests/openid_connect_spec.rb b/spec/requests/openid_connect_spec.rb index bcb8d6c2bfc..b14d4b8fb6e 100644 --- a/spec/requests/openid_connect_spec.rb +++ b/spec/requests/openid_connect_spec.rb @@ -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,41 +94,35 @@ 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 Gitlab root URL' do - expect(@payload['iss']).to eq Gitlab.config.gitlab.url + it 'includes the subject claims' do + expect(@payload).to match(a_hash_including(id_token_claims)) end - it 'includes the hashed user ID' do - expect(@payload['sub']).to eq hashed_subject + it 'includes the Gitlab root URL' do + expect(@payload['iss']).to eq Gitlab.config.gitlab.url end it 'includes the time of the last authentication', :clean_gitlab_redis_shared_state do @@ -118,7 +130,7 @@ describe 'OpenID Connect requests' do 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 |