Skip to content

Geo: Fix push to secondary over SSH for LFS

blocked on gitlab-shell!254 (merged) - Include LFS operation when requesting auth

blocked on https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/8353 - Move JSONWebToken::HMACToken out from EE

What does this MR do?

This MR fixes a bug when pushing to a secondary over SSH for LFS. GitLab supports LFS authentication over SSH (despite the bulk of actions occurring over HTTP) but in the case of pushing to a secondary, the incorrect JSON auth content was returned.

When pushing over SSH to a secondary with git-lfs enabled, the JSON auth blob that is returned from POST /internal/lfs_authenticate, contains credentials & href designed for the secondary. The fragment needs to contain credentials & href designed for the primary.

e.g.

Primary

upload

$ ssh -p 2221 -- ash@ee-secondary1.local git-lfs-authenticate root/test.git upload
{"header":{"Authorization":"Basic abc123"},"href":"https://ee-primary.local/root/test.git/info/lfs/"}

Secondary

upload

$ ssh -p 2222 -- ash@ee-secondary1.local git-lfs-authenticate root/test.git upload
{"header":{"Authorization":"Basic def456"},"href":"https://ee-secondary1.local/root/test.git/info/lfs/"}

It's also worth noting, that when downloading LFS objects, we should still return the secondary URL to keep the operation as fast as possible:

download

$ ssh -p 2222 -- ash@ee-secondary1.local git-lfs-authenticate root/test.git download
{"header":{"Authorization":"Basic def456"},"href":"https://ee-secondary1.local/root/test.git/info/lfs/"}

To work correctly, the secondary must return the same JSON auth payload as the primary. To achieve this, the GitLab::LfsToken#token logic needed to be changed from using a random string (Devise.friendly_token(50)) to use JSONWebToken::HMACToken, based on either the last 16 characters of the user's encrypted password or the first 16 characters of the deploy key / key fingerprint.

I also added missing specs which now brings the coverage up to 100% :)

What are the relevant issue numbers?

https://gitlab.com/gitlab-org/gitlab-ee/issues/8114

Does this MR meet the acceptance criteria?

Edited by Ash McKenzie

Merge request reports