Commit 3ccfff27 authored by Valery Sizov's avatar Valery Sizov

Reduce the scope of Geo JWT (JSON Web Tokens). Step 1

Currently, we generate a large number of JWTs
for Geo - one per file synced, per secondary.
They are short-lived but do not have any restrictions
on scope, so a token valid for downloading one file
or repo could, in theory, be used to download another.

In this commit we add scope to every token but not
enforce it yet.
parent 5a2eef8d
...@@ -265,6 +265,13 @@ questions from [owasp.org](https://www.owasp.org). ...@@ -265,6 +265,13 @@ questions from [owasp.org](https://www.owasp.org).
### What session management requirements have been defined? ### What session management requirements have been defined?
- Geo JWTs are defined to last for only two minutes before needing to be regenerated. - Geo JWTs are defined to last for only two minutes before needing to be regenerated.
- Geo JWTs are generated for one of the following specific scopes:
- Geo API access.
- Git access.
- LFS and File ID.
- Upload and File ID.
- Job Artifact and File ID.
- Geo JWTs scopes are not enforced, but will be in a future version (currently scheduled for GitLab 11.9).
### What access requirements have been defined for URI and Service calls? ### What access requirements have been defined for URI and Service calls?
......
...@@ -103,10 +103,14 @@ module Geo ...@@ -103,10 +103,14 @@ module Geo
# Build a JWT header for authentication # Build a JWT header for authentication
def jwt_authentication_header def jwt_authentication_header
authorization = ::Gitlab::Geo::RepoSyncRequest.new.authorization authorization = ::Gitlab::Geo::RepoSyncRequest.new(scope: gl_repository).authorization
{ "http.#{remote_url}.extraHeader" => "Authorization: #{authorization}" } { "http.#{remote_url}.extraHeader" => "Authorization: #{authorization}" }
end end
def gl_repository
"#{type}-#{project.id}"
end
def remote_url def remote_url
Gitlab::Geo.primary_node.url + repository.full_path + '.git' Gitlab::Geo.primary_node.url + repository.full_path + '.git'
end end
...@@ -126,7 +130,7 @@ module Geo ...@@ -126,7 +130,7 @@ module Geo
temp_repo.create_from_snapshot( temp_repo.create_from_snapshot(
::Gitlab::Geo.primary_node.snapshot_url(temp_repo), ::Gitlab::Geo.primary_node.snapshot_url(temp_repo),
::Gitlab::Geo::RepoSyncRequest.new.authorization ::Gitlab::Geo::RepoSyncRequest.new(scope: ::Gitlab::Geo::API_SCOPE).authorization
) )
rescue => err rescue => err
log_error('Snapshot attempt failed', err) log_error('Snapshot attempt failed', err)
......
...@@ -52,7 +52,7 @@ module Geo ...@@ -52,7 +52,7 @@ module Geo
end end
def headers def headers
Gitlab::Geo::BaseRequest.new.headers Gitlab::Geo::BaseRequest.new(scope: ::Gitlab::Geo::API_SCOPE).headers
end end
def timeout def timeout
......
...@@ -14,6 +14,8 @@ module Gitlab ...@@ -14,6 +14,8 @@ module Gitlab
oauth_application oauth_application
).freeze ).freeze
API_SCOPE = 'geo_api'
def self.current_node def self.current_node
self.cache_value(:current_node, as: GeoNode) { GeoNode.current_node } self.cache_value(:current_node, as: GeoNode) { GeoNode.current_node }
end end
......
...@@ -22,7 +22,9 @@ module Gitlab ...@@ -22,7 +22,9 @@ module Gitlab
{ {
id: upload.model_id, id: upload.model_id,
type: upload.model_type, type: upload.model_type,
checksum: upload.checksum checksum: upload.checksum,
file_type: @file_type,
file_id: @file_id
} }
end end
end end
......
...@@ -17,7 +17,11 @@ module Gitlab ...@@ -17,7 +17,11 @@ module Gitlab
private private
def job_artifact_request_data(job_artifact) def job_artifact_request_data(job_artifact)
{ id: @file_id } {
id: @file_id,
file_type: @file_type,
file_id: @file_id
}
end end
end end
end end
......
...@@ -17,7 +17,11 @@ module Gitlab ...@@ -17,7 +17,11 @@ module Gitlab
private private
def lfs_request_data(lfs_object) def lfs_request_data(lfs_object)
{ checksum: lfs_object.oid } {
checksum: lfs_object.oid,
file_type: @file_type,
file_id: @file_id
}
end end
end end
end end
......
...@@ -15,7 +15,9 @@ describe Gitlab::Geo::FileTransfer do ...@@ -15,7 +15,9 @@ describe Gitlab::Geo::FileTransfer do
expect(Pathname.new(subject.filename).absolute?).to be_truthy expect(Pathname.new(subject.filename).absolute?).to be_truthy
expect(subject.request_data).to eq({ id: upload.model_id, expect(subject.request_data).to eq({ id: upload.model_id,
type: 'User', type: 'User',
checksum: upload.checksum }) checksum: upload.checksum,
file_id: upload.id,
file_type: :file })
end end
end end
end end
......
...@@ -18,7 +18,10 @@ describe Gitlab::Geo::JobArtifactTransfer, :geo do ...@@ -18,7 +18,10 @@ describe Gitlab::Geo::JobArtifactTransfer, :geo do
end end
it 'sets request_data with file_id and file_type' do it 'sets request_data with file_id and file_type' do
expect(described_class.new(job_artifact).request_data).to eq(id: job_artifact.id) expect(described_class.new(job_artifact).request_data).to eq(
id: job_artifact.id,
file_id: job_artifact.id,
file_type: :job_artifact)
end end
end end
end end
...@@ -13,6 +13,10 @@ describe API::Geo do ...@@ -13,6 +13,10 @@ describe API::Geo do
{ 'X-Gitlab-Token' => secondary_node.system_hook.token } { 'X-Gitlab-Token' => secondary_node.system_hook.token }
end end
let(:not_found_req_header) do
Gitlab::Geo::TransferRequest.new(transfer.request_data.merge(file_id: 100000)).headers
end
shared_examples 'with terms enforced' do shared_examples 'with terms enforced' do
before do before do
enforce_terms enforce_terms
...@@ -62,7 +66,7 @@ describe API::Geo do ...@@ -62,7 +66,7 @@ describe API::Geo do
context 'attachment does not exist' do context 'attachment does not exist' do
it 'responds with 404' do it 'responds with 404' do
get api("/geo/transfers/attachment/100000"), headers: req_header get api("/geo/transfers/attachment/100000"), headers: not_found_req_header
expect(response).to have_gitlab_http_status(404) expect(response).to have_gitlab_http_status(404)
end end
...@@ -101,7 +105,7 @@ describe API::Geo do ...@@ -101,7 +105,7 @@ describe API::Geo do
context 'avatar does not exist' do context 'avatar does not exist' do
it 'responds with 404' do it 'responds with 404' do
get api("/geo/transfers/avatar/100000"), headers: req_header get api("/geo/transfers/avatar/100000"), headers: not_found_req_header
expect(response).to have_gitlab_http_status(404) expect(response).to have_gitlab_http_status(404)
end end
...@@ -154,7 +158,7 @@ describe API::Geo do ...@@ -154,7 +158,7 @@ describe API::Geo do
context 'when the Upload record does not exist' do context 'when the Upload record does not exist' do
it 'responds with 404' do it 'responds with 404' do
get api("/geo/transfers/file/100000"), headers: req_header get api("/geo/transfers/file/100000"), headers: not_found_req_header
expect(response).to have_gitlab_http_status(404) expect(response).to have_gitlab_http_status(404)
end end
...@@ -163,10 +167,8 @@ describe API::Geo do ...@@ -163,10 +167,8 @@ describe API::Geo do
describe 'GET /geo/transfers/lfs/1' do describe 'GET /geo/transfers/lfs/1' do
let(:lfs_object) { create(:lfs_object, :with_file) } let(:lfs_object) { create(:lfs_object, :with_file) }
let(:req_header) do let(:transfer) { Gitlab::Geo::LfsTransfer.new(lfs_object) }
transfer = Gitlab::Geo::LfsTransfer.new(lfs_object) let(:req_header) { Gitlab::Geo::TransferRequest.new(transfer.request_data).headers }
Gitlab::Geo::TransferRequest.new(transfer.request_data).headers
end
before do before do
allow_any_instance_of(Gitlab::Geo::TransferRequest).to receive(:requesting_node).and_return(secondary_node) allow_any_instance_of(Gitlab::Geo::TransferRequest).to receive(:requesting_node).and_return(secondary_node)
...@@ -207,7 +209,7 @@ describe API::Geo do ...@@ -207,7 +209,7 @@ describe API::Geo do
context 'LFS object does not exist' do context 'LFS object does not exist' do
it 'responds with 404' do it 'responds with 404' do
get api("/geo/transfers/lfs/100000"), headers: req_header get api("/geo/transfers/lfs/100000"), headers: not_found_req_header
expect(response).to have_gitlab_http_status(404) expect(response).to have_gitlab_http_status(404)
end end
...@@ -216,7 +218,7 @@ describe API::Geo do ...@@ -216,7 +218,7 @@ describe API::Geo do
end end
describe 'POST /geo/status', :postgresql do describe 'POST /geo/status', :postgresql do
let(:geo_base_request) { Gitlab::Geo::BaseRequest.new } let(:geo_base_request) { Gitlab::Geo::BaseRequest.new(scope: ::Gitlab::Geo::API_SCOPE) }
let(:data) do let(:data) do
{ {
......
...@@ -14,7 +14,7 @@ describe API::ProjectSnapshots do ...@@ -14,7 +14,7 @@ describe API::ProjectSnapshots do
end end
it 'requests project repository raw archive from Geo primary as Geo secondary' do it 'requests project repository raw archive from Geo primary as Geo secondary' do
req = Gitlab::Geo::BaseRequest.new req = Gitlab::Geo::BaseRequest.new(scope: ::Gitlab::Geo::API_SCOPE)
allow(req).to receive(:requesting_node) { secondary } allow(req).to receive(:requesting_node) { secondary }
get api("/projects/#{project.id}/snapshot", nil), params: {}, headers: req.headers get api("/projects/#{project.id}/snapshot", nil), params: {}, headers: req.headers
......
...@@ -12,7 +12,7 @@ describe "Git HTTP requests (Geo)" do ...@@ -12,7 +12,7 @@ describe "Git HTTP requests (Geo)" do
set(:secondary) { create(:geo_node) } set(:secondary) { create(:geo_node) }
# Ensure the token always comes from the real time of the request # Ensure the token always comes from the real time of the request
let!(:auth_token) { Gitlab::Geo::BaseRequest.new.authorization } let!(:auth_token) { Gitlab::Geo::BaseRequest.new(scope: "repository-#{project.id}").authorization }
let!(:user) { create(:user) } let!(:user) { create(:user) }
let!(:user_without_any_access) { create(:user) } let!(:user_without_any_access) { create(:user) }
let!(:user_without_push_access) { create(:user) } let!(:user_without_push_access) { create(:user) }
......
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