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.

Sign up for GitHub

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

Jump to bottom

Support 401 responses for Batch API #415

Merged
merged 14 commits into from
Jun 29, 2015
Merged

Support 401 responses for Batch API #415

merged 14 commits into from
Jun 29, 2015

Conversation

rubyist
Copy link
Contributor

@rubyist rubyist commented Jun 18, 2015

This PR adds full 401 support for the Batch API.

  • If access is not marked private, request Batch with no creds
  • If access is marked private, request Batch with creds
  • If a 401 is received, mark private, get creds, and reattempt the request

Related test server PR: git-lfs/lfs-test-server#33

@rubyist rubyist mentioned this pull request Jun 18, 2015
Merged
@technoweenie technoweenie mentioned this pull request Jun 18, 2015
Closed
12 tasks
@technoweenie
Copy link
Contributor

Don't worry about documentation, I already did that here. It's a PR, so still subject to changes of course.

technoweenie
technoweenie reviewed Jun 18, 2015
View reviewed changes
lfs/config.go
@@ -117,6 +117,15 @@ func (c *Configuration) BatchTransfer() bool {
return false
}

func (c *Configuration) PrivateAccess() bool {
if v, ok := c.GitConfig("lfs.access"); ok {
if v == "private" || v == "PRIVATE" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to use strings.ToLower() here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'll fix up nitpicks when things are closer to working. Just wanted to get this linked to your roadmap.

@technoweenie technoweenie mentioned this pull request Jun 19, 2015
Closed
13 tasks
@rubyist rubyist added review and removed wip labels Jun 24, 2015
@rubyist
Copy link
Contributor Author

rubyist commented Jun 24, 2015

This is ready for some 👀. The gist of things:

A copy of the newApiRequest and newClientRequest code was made for batch operations (the old code will eventually go away, so I'm OK with the copy).

When a batch request is run, it will check for an lfs.<url>.access configuration key, where <url> is the endpoint url. If the value is set to "private", the request will run with the credential helpers as it normally has. If the value is missing or any other value, the request will be attempted without authorization.

If a request fails with a 401, the lfs.<url>.access key will be set to private in .git/config, the credentials will be retrieved, and the request will be retried. All future batch requests will then get the credentials when they're run.

@technoweenie technoweenie mentioned this pull request Jun 28, 2015
Closed
rubyist added a commit that referenced this pull request Jun 29, 2015
@rubyist
Merge pull request #415 from github/401batch
9783bff 
Support 401 responses for Batch API
@rubyist rubyist merged commit 9783bff into master Jun 29, 2015
@rubyist rubyist deleted the 401batch branch June 29, 2015 16:13
@rubyist rubyist removed the review label Jun 29, 2015
@technoweenie technoweenie mentioned this pull request Jul 22, 2015
Closed
11 tasks
@Yopai Yopai mentioned this pull request Aug 15, 2015
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
storage
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@rubyist @technoweenie