Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LFS pull fails on public SSH-cloned repo #5478

Closed
2 of 7 tasks
flashka07 opened this issue Dec 5, 2018 · 18 comments · Fixed by #6993
Closed
2 of 7 tasks

LFS pull fails on public SSH-cloned repo #5478

flashka07 opened this issue Dec 5, 2018 · 18 comments · Fixed by #6993
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented type/bug

Comments

@flashka07
Copy link

  • Gitea version (or commit ref): f17524b
  • Git version: 2.17.1
  • Operating system: official docker image
  • Database (use [x]):
    • PostgreSQL
    • MySQL
    • MSSQL
    • SQLite
  • Can you reproduce the bug at https://try.gitea.io:
    • Yes (provide example URL)
    • No
    • Not relevant
  • Log gist:

The problem is that one can't even clone a repo with LFS over SSH if the repo isn't private.
'GIT_TRACE=1 GIT_CURL_VERBOSE=1 git lfs pull'
fails with
'trace git-lfs: api error: Authentication required: Authorization error: https://example.com/gitea/user/repo.git/info/lfs/objects/batch', which is indeed 'HTTP/1.1 401 Unauthorized'.

It is the case because this fragment

		userID, ok := claims["user"].(float64)
		if !ok {
			return nil, r, opStr, fmt.Errorf("Token user id invalid")
		}

in modules/lfs/server.go (parseToken) gives an error.

From the other side command
'ssh -- [email protected] git-lfs-authenticate user/repo.git download'
returns auth token without 'user' field because this condition

if requestedMode == models.AccessModeWrite || repo.IsPrivate || setting.Service.RequireSignInView

in cmd/serv.go (runServ) is false. It is even false if one has 'REQUIRE_SIGNIN_VIEW = true' in his config, because noone initializes setting.Service.RequireSignInView (setting.newService() doesn't get called I guess).
Changing repo type to private solves the problem, but here is the bug anyway.
I also think that initalizing RequireSignInView should be performed, but this is not a proper solution to the issue, since HTTPS cloning of public repo works fine with the same settings.

@cnzgray
Copy link

cnzgray commented Dec 6, 2018

I have the same problem.

@techknowlogick techknowlogick added the issue/duplicate The issue has already been reported. label Dec 6, 2018
@techknowlogick
Copy link
Member

Closing as duplicate of #2475

@cnzgray
Copy link

cnzgray commented Dec 7, 2018

I think this is not the same as #2475.

The problem is that using git-lfs-authenticate will get the correct response in the private repository.

In the public repository, but its own gitea's REQUIRE_SIGNIN_VIEW=true, an unauthorized exception was raised.

@techknowlogick
Copy link
Member

@cnzgray my understanding of this issue is that LFS fails when trying to clone via SSH, because LFS isn't supported under SSH per linked issue. So I think that the linked issue should be solved first because then this issue might not even exist. @flashka07 is free to open this issue again, and if so I will relabel it as bug

@cnzgray
Copy link

cnzgray commented Dec 7, 2018

@techknowlogick You are right, the essence of the problem is the transmission of LFS under SSH. However, for LFS related issues under SSH, there is still no complete point in time.

The current gitea support for git-lfs-authenticate is correct (git clone uses ssh, LFS uses http), there is only one bug.

When the gitea site is set to be authorized to access, the token of the public repository in the site cannot be correctly identified.

thank you very much.

@techknowlogick
Copy link
Member

That sounds reasonable, thank you for taking the time to explain. I will re-open.

@techknowlogick techknowlogick reopened this Dec 7, 2018
@techknowlogick techknowlogick added type/bug and removed issue/duplicate The issue has already been reported. labels Dec 7, 2018
@flashka07
Copy link
Author

@cnzgray exactly, thank you.
I think it is reasonable to add user id to the token in any case (either on download or upload action), if the git-lfs-authenticate is being issued via ssh (I don't know if it can be issued by someone not via ssh), because there is no anonymous ssh access at all.
It will solve both mentioned problems (ignorance of REQUIRE_SIGNIN_VIEW and cloning public repositories without 'sign-in' restriction).

@stale
Copy link

stale bot commented Feb 9, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 weeks. Thank you for your contributions.

@stale stale bot added the issue/stale label Feb 9, 2019
@lunny
Copy link
Member

lunny commented Feb 9, 2019

Can not reproduce this. Is this still a problem?

@stale stale bot removed the issue/stale label Feb 9, 2019
@stale
Copy link

stale bot commented Apr 10, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 weeks. Thank you for your contributions.

@stale stale bot added the issue/stale label Apr 10, 2019
@ghost
Copy link

ghost commented Apr 19, 2019

this is still relevant, i could not clone with ssh but once LFS was disabled it bypassed the error

edit: but pushing doesn't work with this workaround

@stale stale bot removed the issue/stale label Apr 19, 2019
@lunny lunny added the issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented label Apr 19, 2019
@zeripath
Copy link
Contributor

@laerus could you very quickly give me a minimal test case for this. (I appreciate you can't replicate on try because LFS is turned off.)

@zeripath
Copy link
Contributor

zeripath commented Apr 19, 2019

can you reproduce on the latest docker?

@ghost
Copy link

ghost commented May 3, 2019

@zeripath i've updated to v1.8 and this is still an issue, cannot ssh clone when LFS is enabled and if i disable LFS cloning works but pushing doesn't

@zeripath
Copy link
Contributor

zeripath commented May 3, 2019

Give me a minimal testcase to reproduce and I'll see what I can do. At present I just don't have enough information to figure out what's going wrong and I can't reproduce it.

@ghost
Copy link

ghost commented May 6, 2019

@zeripath atm i can only reproduce this on the production machine and not on a clean gitea deployment. I will have to do some more digging, if you have any tips on how to debug this i may be able to provide more info.

@zeripath
Copy link
Contributor

zeripath commented May 6, 2019

Hmm... that makes me suspicious that this may be something to do with the setup of that machine. Perhaps proxy settings or something else.

I see you are mounting your gitea as a suburl of a domain? I wonder if there's (another) bug with that. When you try to duplicate are you also mounting as a sub-domain?

nopjmp added a commit to nopjmp/gitea that referenced this issue May 12, 2019
This should fix go-gitea#5478 and with REQUIRE_SIGNIN_VIEW=true lfs auth errors
@nopjmp
Copy link

nopjmp commented May 18, 2019

It was mentioned by @zeripath in the #6916 that the two commands cmd/serv and cmd/dump should be modified in order to do internal api requests instead of doing the work themselves. This would allow all internal gitea logging and settings to be centralized to avoid logging to multiple places.

My minimum test case is to make a "public" repo with REQUIRE_SIGNIN_VIEW set to true, and commit at least 1 LFS file.

Thus the following:
requestedMode == models.AccessModeWrite || repo.IsPrivate || setting.Service.RequireSignInView

This is evaluated as false since the requestedMode is Read, repo.IsPrivate is false, and setting.Service.RequireSignInView is false due to the settings not being initialized.

My fix was to initialize the settings like cmd/dump did, but after taking some time to think, I agree with @lunny that this is not the way to fix it and @zeripath idea is best.

zeripath added a commit to zeripath/gitea that referenced this issue Jun 16, 2019
jolheiser pushed a commit to jolheiser/gitea that referenced this issue Jun 17, 2019
* Always set userID on LFS authentication

Fix go-gitea#5478
Fix go-gitea#7219

* Deploy keys should only be able to read their repos
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented type/bug
Projects
None yet
6 participants