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

Github authentication #166

Open
SkyDev125 opened this issue Jul 30, 2024 · 15 comments · May be fixed by #175
Open

Github authentication #166

SkyDev125 opened this issue Jul 30, 2024 · 15 comments · May be fixed by #175

Comments

@SkyDev125
Copy link

I''ve been trying to understand how to do the github auth while using my giftless docker container...

But from what I''m understanding you cant specifically just limit it so giftless only allows people from a specific repository to upload/download the files...

Which theorically means that... if I just create a repo of my own.. being a complete random.. and I know the IP of the giftless server. I configure it to use the giftless server. and done. I''m literally bypassing all authentication, cause I'm the owner of my own repo... and now I can use this server and abuse of it as much as I'd like...

Is there a way to use github... but to limit the authentication, the ability to use the entirety of the giftless private server. only for certain people in a specific github organization????

@vit-zikmund
Copy link
Contributor

Hi @SkyDev125 it seems like your observation is correct. I didn't develop the auth provider with this consideration, because our LFS server is behind a VPN, so no stranger can come and do this. I don't think anyone in their right mind would want to store their work there like that, but as a temporary free storage for pretty much anything, well that's a different song! So it's a perfectly valid concern, which should be fixed.

Hooking such a filter to the current implementation should be pretty easy. The provider is already properly configurable and the filter can go right after the place the github user is resolved. At that moment one also has the information about the organization, repo and the user. There's no information what org/group the user is a member of. This would require yet another call to the github API, which I'd like to avoid.

Giving it a second thought, a very cheap and effective filter would be to target an organization and/or its repos. This way you wouldn't need to do any extra api call and could reject outsiders even before initially resolving the token. Would that make sense to you?

One could configure it like this:

restrict_to:
  org1:
  - repo1  # accept org1/repo1
  - repo2  # accept org1/repo2
  org2: []  # accept all repos of that org

I think filtering for users would only bring more burden to the Giftless admin, while that info should be naturally already present in the access roles on the targeted github repo.

@SkyDev125
Copy link
Author

No, definitely I agree with you

If we could do that configuration, as you wrote

restricting the access to the gitfless so only people from a specific organization, or a repo in an organization seems to be the right way to do this.

Not only it makes everything more secure. But also viable for people to expose the server and use it as expected.

@SkyDev125
Copy link
Author

So long as this doesnt limit me from still making the server readonly for the general folk.

So in case someone from outside the organization tries to clone a repo, they'd still be able to get the contents as expected and work on the project to then submit a pull request... as the normal github colaboration process goes.

@vit-zikmund
Copy link
Contributor

Fancy to up your mad python skillz ™️ on a PR? 😉 I'm kinda swamped with my daily job these days.

@SkyDev125
Copy link
Author

Well.. I guess I can try... just gotta read up the docs a bit more... and try to understand what really is going on. but sure.

@vit-zikmund
Copy link
Contributor

So in case someone from outside the organization tries to clone a repo, they'd still be able to get the contents as expected and work on the project to then submit a pull request... as the normal github colaboration process goes.

Hmm, there are a couple other obstacles here:

  • currently, the github auth plugin doesn't check whether the repo is public (where naturally the world has read access), so it won't give read access to LFS of a normally readable repo. This seems like something that should be fixed, too. At least for the case the user is not found in the repo collabolators.
  • Submitting a PR to a world-readable repo containing LFS files can work, but here it gets tricky.
    • The submitter will likely need their own LFS (unless you share yours beforehand)
    • You, doing the merge, will need read access to it.
    • GitHub will surely let you do the merge via the web UI, but then you'll end up with LFS blob references on your repo that are missing on your LFS server. So the only meaningful way is doing such a merge locally, first pulling all the blobs in play from your and the submitter's LFS server(s), merging it and finally pushing all that to your LFS.
    • Cleaning one's LFS after a PR branch has been merged/deleted is also a thing to consider, for which git lfs has so far no interface (AFAIK).

@vit-zikmund
Copy link
Contributor

vit-zikmund commented Aug 2, 2024

For the record, checking if a repo is readable would be done with the Get a repository API call and checking the visibility field for public or internal.
public is simply readable to the world, internal only to members of GitHub Enterprise organization. I assume the latter wouldn't be visible at all for a user outside of the enterprise organization, so a 40x response is a good marker the repo is not readable.

@SkyDev125
Copy link
Author

I am just surprised how in 2024 we still struggle to have a system that can handle large files properly... I'd have hoped that it would have gone further than we are. everything seems so... underdeveloped.

I guess this isnt a problem that most developers have, hence why the lack of development in solutions for this

Giftless was pretty much the only one I found that could do somewhat what I intended...

But yeah thank you for pointing that out. I guess I need to think about this more than I thought

Perhaps just making it work for the people inside the organization should be the only thing to worry about right now...

@vit-zikmund
Copy link
Contributor

vit-zikmund commented Aug 5, 2024

Last year, I was in your position, with the only difference - there was nothing. As the open-source world goes, when there's nothing, you go and create something. I'm not at all affiliated with Datopian, but I wanted that feature and it turned out I can craft it out, I was just lucky to meet another enthusiast dev here (also not Datopian) who helped me wrap up the thing as it stands now. (Sadly, Datopian has some internal problems doing further releases of Giftless I really don't understand. but at least the main branch is in a very decent state.)

Ranting about underdevelopment will get you nowhere, it's only the dedication of folks to drive things forward. All these projects need your help, you know. Most of this work isn't paid at all and when you enter the adult world it's no surprise one switches from their previous passions for some more mundane things... If there's a tool you can almost fit to your needs and can't fit your needs fully around the tool, you gotta back off or improve the tool 😛 Both the changes you desire are something like 20 lines (+ tests) of extending the code that's already there, you don't need to be afraid 💪

And if we're talking about underdevelopment, look here. Trying to debug the github auth provider I realized the git-lfs client has a very nasty behavior regarding handling errors, e.g. immediately retrying (forever) on a 403 Forbidden response from the LFS server, which should be a dead stop for it, but your only hope in fending off its hammering is your front proxy eventually returning a 503 Service Unavailable, when the LFS server can't catch its breath.

@SkyDev125
Copy link
Author

It's just... that it is my first pull request...

I know how to usually do projects start to finish and manage to make something that works.. But I've never before tried to modify someone else's code.. at least not at the complexity that this is..

Hence why the somewhat insecurities I have over here... It's just my first time.. I barely even know how to do a pull request on my own >.<

But well, I'll research and try.

@vit-zikmund
Copy link
Contributor

vit-zikmund commented Aug 5, 2024

No worries one has to start somewhere ;) Here's a little help:

  • First of all do a private fork of this repo (this will be the place for y our changes - and the source of the PR)
  • Clone it to your machine (not the original repo, as you won't be able to push any changes there)
  • Learn a bit about the local dev env (this doc is a little outdated, but still good to get oriented)
  • Learn how to run the whole thing locally
  • ./flask-develop.sh implicitly passes a giftless.yaml to the runtime, so feel free to fill your desired config there.
  • Learn about git-lfs /batch API, so you can craft a testing curl message to test your changes.
  • When your changes are to your liking, don't hesitate to open a PR (draft, if you want to ask for some directions) against the original project, feel free to tag me, so I'd get notified (I think the locals will be thrilled to let me review that 😇)

@SkyDev125
Copy link
Author

Thank you for the directions, I've already set-up the project and am running the default tests just to make sure everything is alright.

Will start now doing some of the research, I really appreciate the help.

@vit-zikmund
Copy link
Contributor

Hey @SkyDev125, I was in need of doing some beefy updates and thought this little change might hitch a ride, too.
As my big PR has not yet been noticed, I won't PR this change (as it would likely clash, as I made it on top of the PR'd changes). Anyways, the change is in my personal fork: vit-zikmund@b191730, which I will PR once some good soul works through the first one.

@rufuspollock
Copy link
Member

@SkyDev125 welcome! And the dream of open source is going to come true for you it seems 😄 as @vit-zikmund amazing work will have you covered. We are working on getting #169 merged ...

@SkyDev125
Copy link
Author

I wish I hadn't got all caught up with uni and that had been able to actually have worked on this.

But we'll, there will be some other opportunities :p I am happy to see progress here though!

Thanks for the updates :3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants