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

Update NewClient handle credentials in the url #23

Merged
merged 12 commits into from
Oct 25, 2016
Merged

Conversation

opalmer
Copy link
Contributor

@opalmer opalmer commented Oct 15, 2016

This PR updates NewClient to handle credentials in the url:

client, err := gerrit.NewClient("http://user:secret@host:port/")

If the credentials are invalid then err will be set appropriately. This new function will be used in the docker_tests branch.

Another case were this could be useful is when a consumer of go-gerrit is using configuration files to drive their application. In these cases code could go from:

username, password := GetCredentials()
client, err := gerrit.NewClient(GetURL())
client.Authentication.SetDigestAuth(username, password)
// test auth

To:

client, err := gerrit.NewClient(GetURL())
// make sure err == nil

 NewClientFromURL is a wrapper for NewClient with two notable differences:
   * The url may contain credentials, http://admin:secret@localhost:8081/ for
     example. These credentials may either be a user name and password or
     name and value as in the case of cookie based authentication.
   * If the url contains credentials then this function will attempt to validate
     the credentials before returning the client. An error will be returned if the
     credentials cannot be validated.
 The process of validating the credentials is relatively simple and only requires that
 the provided user have permission to GET /a/accounts/self. Internally NewClientFromURL
 calls each of the Set*Auth functions followed by GetAccount("self"). The first call to
 GetAccount("self") which succeeds will return a *Client.
@opalmer opalmer added this to the 0.1.1 milestone Oct 15, 2016
@opalmer opalmer self-assigned this Oct 15, 2016
@opalmer
Copy link
Contributor Author

opalmer commented Oct 16, 2016

@andygrunwald I know you're probably still out so I'll try to get to the point:

  • This PR adds a function (NewClientFromURL) which allows credentials to be provided as part of the url. Other projects I've worked with have similar functions and I've already implemented this same function on another project using go-gerrit (so it would be nice to open source it).
  • There are several tests which cover the new code including url handling, failed authentication and digest/basic/cookie auth too (which previously had no tests).

Unless you have some major objections, I'd like to go ahead an merge this so we can start to use it in the docker tests.

@opalmer opalmer changed the title Implement NewClientFromURL to construct *gerrit.Client with credentials Update NewClient handle credentials in the url Oct 20, 2016
@opalmer
Copy link
Contributor Author

opalmer commented Oct 20, 2016

Minor update, added credential handling directly to NewClient without breaking compatibility.

@coveralls
Copy link

coveralls commented Oct 20, 2016

Coverage Status

Coverage increased (+7.0%) to 17.481% when pulling 4f29c44 on from_url into 2b0558c on master.

@andygrunwald
Copy link
Owner

I like the idea, because it breaks nothing :)
I will merge it. Thanks @opalmer

@andygrunwald andygrunwald merged commit c44a8a8 into master Oct 25, 2016
@andygrunwald andygrunwald deleted the from_url branch October 25, 2016 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants