From dfd0e2450aabc3b5c322c4a4382edb84caa7101b Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Sun, 15 May 2016 08:52:26 -0500 Subject: [PATCH] Improve authentication service specs --- ...ntainer_registry_authentication_service.rb | 6 +-- spec/lib/json_web_token/rsa_token_spec.rb | 28 ++++++++---- ...er_registry_authentication_service_spec.rb | 44 +++++++++++++++---- 3 files changed, 59 insertions(+), 19 deletions(-) diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb index bbbc84475c8..c61d339ffdd 100644 --- a/app/services/auth/container_registry_authentication_service.rb +++ b/app/services/auth/container_registry_authentication_service.rb @@ -7,10 +7,10 @@ module Auth if params[:offline_token] return error('forbidden', 403) unless current_user + else + return error('forbidden', 401) unless scope end - return error('forbidden', 401) unless scope - { token: authorized_token(scope).encoded } end @@ -21,7 +21,7 @@ module Auth token.issuer = registry.issuer token.audience = params[:service] token.subject = current_user.try(:username) - token[:access] = accesses + token[:access] = accesses.compact token end diff --git a/spec/lib/json_web_token/rsa_token_spec.rb b/spec/lib/json_web_token/rsa_token_spec.rb index 4462cdde9a3..0c3d3ea7019 100644 --- a/spec/lib/json_web_token/rsa_token_spec.rb +++ b/spec/lib/json_web_token/rsa_token_spec.rb @@ -1,5 +1,17 @@ describe JSONWebToken::RSAToken do - let(:rsa_key) { generate_key } + let(:rsa_key) do + OpenSSL::PKey::RSA.new <<-eos.strip_heredoc + -----BEGIN RSA PRIVATE KEY----- + MIIBOgIBAAJBAMA5sXIBE0HwgIB40iNidN4PGWzOyLQK0bsdOBNgpEXkDlZBvnak + OUgAPF+rME4PB0Yl415DabUI40T5UNmlwxcCAwEAAQJAZtY2pSwIFm3JAXIh0cZZ + iXcAfiJ+YzuqinUOS+eW2sBCAEzjcARlU/o6sFQgtsOi4FOMczAd1Yx8UDMXMmrw + 2QIhAPBgVhJiTF09pdmeFWutCvTJDlFFAQNbrbo2X2x/9WF9AiEAzLgqMKeStSRu + H9N16TuDrUoO8R+DPqriCwkKrSHaWyMCIFzMhE4inuKcSywBaLmiG4m3GQzs++Al + A6PRG/PSTpQtAiBxtBg6zdf+JC3GH3zt/dA0/10tL4OF2wORfYQghRzyYQIhAL2l + 0ZQW+yLIZAGrdBFWYEAa52GZosncmzBNlsoTgwE4 + -----END RSA PRIVATE KEY----- + eos + end let(:rsa_token) { described_class.new(nil) } let(:rsa_encoded) { rsa_token.encoded } @@ -13,19 +25,19 @@ describe JSONWebToken::RSAToken do it { expect{subject}.to_not raise_error } it { expect(subject.first).to include('key' => 'value') } + it do + expect(subject.second).to eq( + "typ" => "JWT", + "alg" => "RS256", + "kid" => "OGXY:4TR7:FAVO:WEM2:XXEW:E4FP:TKL7:7ACK:TZAF:D54P:SUIA:P3B2") + end end context 'for invalid key to raise an exception' do - let(:new_key) { generate_key } + let(:new_key) { OpenSSL::PKey::RSA.generate(512) } subject { JWT.decode(rsa_encoded, new_key) } it { expect{subject}.to raise_error(JWT::DecodeError) } end end - - private - - def generate_key - OpenSSL::PKey::RSA.generate(512) - end end diff --git a/spec/services/auth/container_registry_authentication_service_spec.rb b/spec/services/auth/container_registry_authentication_service_spec.rb index a2937368136..4a6cd132e8d 100644 --- a/spec/services/auth/container_registry_authentication_service_spec.rb +++ b/spec/services/auth/container_registry_authentication_service_spec.rb @@ -57,15 +57,28 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do end end - shared_examples 'a forbidden' do + shared_examples 'a unauthorized' do it { is_expected.to include(http_status: 401) } it { is_expected.to_not include(:token) } end + shared_examples 'a forbidden' do + it { is_expected.to include(http_status: 403) } + it { is_expected.to_not include(:token) } + end + context 'user authorization' do let(:project) { create(:project) } let(:current_user) { create(:user) } + context 'allow to use offline_token' do + let(:current_params) do + { offline_token: true } + end + + it_behaves_like 'an authenticated' + end + context 'allow developer to push images' do before { project.team << [current_user, :developer] } @@ -103,13 +116,21 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do { scope: "repository:#{project.path_with_namespace}:pull,push" } end - it_behaves_like 'a forbidden' + it_behaves_like 'a unauthorized' end end context 'project authorization' do let(:current_project) { create(:empty_project) } + context 'disallow to use offline_token' do + let(:current_params) do + { offline_token: true } + end + + it_behaves_like 'a forbidden' + end + context 'allow to pull and push images' do let(:current_params) do { scope: "repository:#{current_project.path_with_namespace}:pull,push" } @@ -133,7 +154,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do context 'disallow for private' do let(:project) { create(:empty_project, :private) } - it_behaves_like 'a forbidden' + it_behaves_like 'a unauthorized' end end @@ -144,20 +165,27 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do context 'disallow for all' do let(:project) { create(:empty_project, :public) } - it_behaves_like 'a forbidden' + it_behaves_like 'a unauthorized' end end - end end context 'unauthorized' do + context 'disallow to use offline_token' do + let(:current_params) do + { offline_token: true } + end + + it_behaves_like 'a forbidden' + end + context 'for invalid scope' do let(:current_params) do { scope: 'invalid:aa:bb' } end - it_behaves_like 'a forbidden' + it_behaves_like 'a unauthorized' end context 'for private project' do @@ -167,7 +195,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do { scope: "repository:#{project.path_with_namespace}:pull" } end - it_behaves_like 'a forbidden' + it_behaves_like 'a unauthorized' end context 'for public project' do @@ -186,7 +214,7 @@ describe Auth::ContainerRegistryAuthenticationService, services: true do { scope: "repository:#{project.path_with_namespace}:push" } end - it_behaves_like 'a forbidden' + it_behaves_like 'a unauthorized' end end end