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

Add basic auth support: Sling.SetBasicAuth() #16

Closed
wants to merge 3 commits into from

Conversation

AaronO
Copy link
Contributor

@AaronO AaronO commented Jun 8, 2016

Sling is one of the nicest libraries I found, it's composable and elegant. However I found it frustrating that it lacked Basic Auth support which is a must have when writing API clients, so I thought I would add here.

Tried to keep it as simple as possible and follow existing conventions (allow chaining etc ...)

Let me know if you have any questions.

@@ -145,6 +146,20 @@ func (s *Sling) Set(key, value string) *Sling {
return s
}

// SetBasicAuth mimics http.Request.SetBasicAuth()
Copy link
Owner

Choose a reason for hiding this comment

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

// SetBasicAuth sets the Authorization header to use HTTP Basic Authentication
// with the provided username and password. With HTTP Basic Authentication
// the provided username and password are not encrypted.

^ let's match net/http so its implicitly clear it does the same.

@dghubble
Copy link
Owner

dghubble commented Jun 9, 2016

Simple enough to do it directly on the http.Request but I can see the desire to use the same chaining (fluent stlye) for this with Sling. Can you add a sanity test with the user: Aladdin, password: "open sesame" example from the RFC please?

Otherwise, looks good for merge. Thanks for the PR.

@AaronO
Copy link
Contributor Author

AaronO commented Jun 13, 2016

@dghubble Voilà, updated comments and added tests.

You can indeed do it on the http.Request but that doesn't feel elegant and is especially awkward when building an API client. Now you can get a full base client that's chainable in one line:

baseClient := sling.New().Base("https://api.example.com").SetBasicAuth("admin", "secret")

So you can set your auth once when building your base client, then forget about it which is quite nice.

Since sling is especially built for interacting with APIs it seemed odd that adding auth (required for all production APIs) was awkward or didn't follow the simplicity and ease of use elsewhere (automatic JSON serialization, etc ...)

@AaronO
Copy link
Contributor Author

AaronO commented Jun 13, 2016

@dghubble Sorry for taking a few days to reply to your feedback. I got swamped with work. Let me know if you have any further questions.

@dghubble
Copy link
Owner

dghubble commented Jun 14, 2016

Squash merged as c1694aa. Thanks!

@dghubble dghubble closed this Jun 14, 2016
@AaronO AaronO deleted the feature/basicauth branch March 11, 2018 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants