From 3b973326fabd98ffaa68dfa7d9f5b7ecb8aa0e54 Mon Sep 17 00:00:00 2001 From: "Jeevanandam M." Date: Mon, 18 Nov 2024 22:39:45 -0800 Subject: [PATCH] feat!: enhance credentials usage and update warning message for credentials requests (#915) - User type become unexported - HTTP not secure warning message added for auth token flow too - Intialize the default auth scheme value `Bearer` during client creation --- client.go | 35 ++++++++--------------------------- client_test.go | 4 ++-- digest.go | 30 +++++++++++++----------------- example_test.go | 5 ++++- middleware.go | 27 +++++++++++---------------- request.go | 12 ++++++------ request_test.go | 31 +++++++++++++++++-------------- resty.go | 1 + resty_test.go | 2 +- util.go | 17 +++++++++++++++++ 10 files changed, 80 insertions(+), 84 deletions(-) diff --git a/client.go b/client.go index f95d5c59..3ea8a364 100644 --- a/client.go +++ b/client.go @@ -76,6 +76,8 @@ var ( jsonKey = "json" xmlKey = "xml" + defaultAuthScheme = "Bearer" + hdrUserAgentValue = "go-resty/" + Version + " (https://github.com/go-resty/resty)" bufPool = &sync.Pool{New: func() any { return &bytes.Buffer{} }} ) @@ -172,7 +174,7 @@ type Client struct { pathParams map[string]string rawPathParams map[string]string header http.Header - userInfo *User + credentials *credentials authToken string authScheme string cookies []*http.Cookie @@ -221,11 +223,6 @@ type Client struct { certWatcherStopChan chan bool } -// User type is to hold an username and password information -type User struct { - Username, Password string -} - // CertWatcherOptions allows configuring a watcher that reloads dynamically TLS certs. type CertWatcherOptions struct { // PoolInterval is the frequency at which resty will check if the PEM file needs to be reloaded. @@ -233,13 +230,6 @@ type CertWatcherOptions struct { PoolInterval time.Duration } -// Clone method returns deep copy of u. -func (u *User) Clone() *User { - uu := new(User) - *uu = *u - return uu -} - //‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾ // Client methods //___________________________________ @@ -498,15 +488,6 @@ func (c *Client) SetFormData(data map[string]string) *Client { return c } -// UserInfo method returns the authorization username and password. -// -// userInfo := client.UserInfo() -func (c *Client) UserInfo() *User { - c.lock.RLock() - defer c.lock.RUnlock() - return c.userInfo -} - // SetBasicAuth method sets the basic authentication header in the HTTP request. For Example: // // Authorization: Basic @@ -522,7 +503,7 @@ func (c *Client) UserInfo() *User { func (c *Client) SetBasicAuth(username, password string) *Client { c.lock.Lock() defer c.lock.Unlock() - c.userInfo = &User{Username: username, Password: password} + c.credentials = &credentials{Username: username, Password: password} return c } @@ -611,8 +592,8 @@ func (c *Client) SetDigestAuth(username, password string) *Client { c.lock.Unlock() c.AddRequestMiddleware(func(c *Client, _ *Request) error { c.httpClient.Transport = &digestTransport{ - digestCredentials: digestCredentials{username, password}, - transport: oldTransport, + credentials: credentials{username, password}, + transport: oldTransport, } return nil }) @@ -638,7 +619,6 @@ func (c *Client) R() *Request { IsTrace: c.isTrace, AuthScheme: c.authScheme, AuthToken: c.authToken, - UserInfo: c.userInfo, RetryCount: c.retryCount, RetryWaitTime: c.retryWaitTime, RetryMaxWaitTime: c.retryMaxWaitTime, @@ -660,6 +640,7 @@ func (c *Client) R() *Request { setContentLength: c.setContentLength, generateCurlOnDebug: c.generateCurlOnDebug, unescapeQueryParams: c.unescapeQueryParams, + credentials: c.credentials, } if c.ctx != nil { @@ -2012,7 +1993,7 @@ func (c *Client) Clone(ctx context.Context) *Client { cc.header = c.header.Clone() cc.pathParams = maps.Clone(c.pathParams) cc.rawPathParams = maps.Clone(c.rawPathParams) - cc.userInfo = c.userInfo.Clone() + cc.credentials = c.credentials.Clone() cc.contentTypeEncoders = maps.Clone(c.contentTypeEncoders) cc.contentTypeDecoders = maps.Clone(c.contentTypeDecoders) cc.contentDecompressors = maps.Clone(c.contentDecompressors) diff --git a/client_test.go b/client_test.go index 5a6ae4de..4e247415 100644 --- a/client_test.go +++ b/client_test.go @@ -1381,8 +1381,8 @@ func TestClientClone(t *testing.T) { // assert non-interface type assertEqual(t, "http://localhost", parent.BaseURL()) assertEqual(t, "https://local.host", clone.BaseURL()) - assertEqual(t, "parent", parent.UserInfo().Username) - assertEqual(t, "clone", clone.UserInfo().Username) + assertEqual(t, "parent", parent.credentials.Username) + assertEqual(t, "clone", clone.credentials.Username) // assert interface/pointer type assertEqual(t, parent.Client(), clone.Client()) diff --git a/digest.go b/digest.go index d112bfd2..5b55dbe3 100644 --- a/digest.go +++ b/digest.go @@ -38,12 +38,8 @@ var hashFuncs = map[string]func() hash.Hash{ "SHA-512-256-sess": sha512.New, } -type digestCredentials struct { - username, password string -} - type digestTransport struct { - digestCredentials + credentials transport http.RoundTripper } @@ -98,9 +94,9 @@ func (dt *digestTransport) RoundTrip(req *http.Request) (*http.Response, error) return dt.transport.RoundTrip(req2) } -func (dt *digestTransport) newCredentials(req *http.Request, c *challenge) *credentials { - return &credentials{ - username: dt.username, +func (dt *digestTransport) newCredentials(req *http.Request, c *challenge) *digestCredentials { + return &digestCredentials{ + username: dt.Username, userhash: c.userhash, realm: c.realm, nonce: c.nonce, @@ -111,7 +107,7 @@ func (dt *digestTransport) newCredentials(req *http.Request, c *challenge) *cred messageQop: c.qop, nc: 0, method: req.Method, - password: dt.password, + password: dt.Password, } } @@ -203,7 +199,7 @@ func parseChallenge(input string) (*challenge, error) { return c, nil } -type credentials struct { +type digestCredentials struct { username string userhash string realm string @@ -219,7 +215,7 @@ type credentials struct { password string } -func (c *credentials) authorize() (string, error) { +func (c *digestCredentials) authorize() (string, error) { if _, ok := hashFuncs[c.algorithm]; !ok { return "", ErrDigestAlgNotSupported } @@ -257,7 +253,7 @@ func (c *credentials) authorize() (string, error) { return fmt.Sprintf("Digest %s", strings.Join(sl, ", ")), nil } -func (c *credentials) validateQop() error { +func (c *digestCredentials) validateQop() error { // Currently only supporting auth quality of protection. TODO: add auth-int support // NOTE: cURL support auth-int qop for requests other than POST and PUT (i.e. w/o body) by hashing an empty string // is this applicable for resty? see: https://github.com/curl/curl/blob/307b7543ea1e73ab04e062bdbe4b5bb409eaba3a/lib/vauth/digest.c#L774 @@ -282,14 +278,14 @@ func (c *credentials) validateQop() error { return nil } -func (c *credentials) h(data string) string { +func (c *digestCredentials) h(data string) string { hfCtor := hashFuncs[c.algorithm] hf := hfCtor() _, _ = hf.Write([]byte(data)) // Hash.Write never returns an error return fmt.Sprintf("%x", hf.Sum(nil)) } -func (c *credentials) resp() (string, error) { +func (c *digestCredentials) resp() (string, error) { c.nc++ b := make([]byte, 16) @@ -306,12 +302,12 @@ func (c *credentials) resp() (string, error) { c.nonce, c.nc, c.cNonce, c.messageQop, ha2)), nil } -func (c *credentials) kd(secret, data string) string { +func (c *digestCredentials) kd(secret, data string) string { return c.h(fmt.Sprintf("%s:%s", secret, data)) } // RFC 7616 3.4.2 -func (c *credentials) ha1() string { +func (c *digestCredentials) ha1() string { ret := c.h(fmt.Sprintf("%s:%s:%s", c.username, c.realm, c.password)) if c.sessionAlg { return c.h(fmt.Sprintf("%s:%s:%s", ret, c.nonce, c.cNonce)) @@ -321,7 +317,7 @@ func (c *credentials) ha1() string { } // RFC 7616 3.4.3 -func (c *credentials) ha2() string { +func (c *digestCredentials) ha2() string { // currently no auth-int support return c.h(fmt.Sprintf("%s:%s", c.method, c.digestURI)) } diff --git a/example_test.go b/example_test.go index ed993f67..0b4eb0b2 100644 --- a/example_test.go +++ b/example_test.go @@ -99,9 +99,12 @@ func Example_post() { printOutput(resp1, err1) + type User struct { + Username, Password string + } // POST Struct, default is JSON content type. No need to set one resp2, err2 := client.R(). - SetBody(resty.User{Username: "testuser", Password: "testpass"}). + SetBody(User{Username: "testuser", Password: "testpass"}). SetResult(&AuthSuccess{}). // or SetResult(AuthSuccess{}). SetError(&AuthError{}). // or SetError(AuthError{}). Post("https://myapp.com/login") diff --git a/middleware.go b/middleware.go index 0f345e2f..0bacf275 100644 --- a/middleware.go +++ b/middleware.go @@ -302,28 +302,23 @@ func createHTTPRequest(c *Client, r *Request) (err error) { } func addCredentials(c *Client, r *Request) error { - var isBasicAuth bool + credentialsAdded := false // Basic Auth - if r.UserInfo != nil { - r.RawRequest.SetBasicAuth(r.UserInfo.Username, r.UserInfo.Password) - isBasicAuth = true - } - - if !c.IsDisableWarn() { - if isBasicAuth && !strings.HasPrefix(r.URL, "https") { - r.log.Warnf("Using Basic Auth in HTTP mode is not secure, use HTTPS") - } + if r.credentials != nil { + credentialsAdded = true + r.RawRequest.SetBasicAuth(r.credentials.Username, r.credentials.Password) } // Build the token Auth header if !isStringEmpty(r.AuthToken) { - var authScheme string - if isStringEmpty(r.AuthScheme) { - authScheme = "Bearer" - } else { - authScheme = r.AuthScheme + credentialsAdded = true + r.RawRequest.Header.Set(c.HeaderAuthorizationKey(), r.AuthScheme+" "+r.AuthToken) + } + + if !c.IsDisableWarn() && credentialsAdded { + if strings.HasPrefix(r.URL, "http") { + r.log.Warnf("Using sensitive credentials in HTTP mode is not secure. Use HTTPS") } - r.RawRequest.Header.Set(c.HeaderAuthorizationKey(), authScheme+" "+r.AuthToken) } return nil diff --git a/request.go b/request.go index f9cf4b39..93b942ed 100644 --- a/request.go +++ b/request.go @@ -47,7 +47,6 @@ type Request struct { Result any Error any RawRequest *http.Request - UserInfo *User Cookies []*http.Cookie Debug bool CloseConnection bool @@ -77,6 +76,7 @@ type Request struct { // first attempt + retry count = total attempts Attempt int + credentials *credentials isMultiPart bool isFormData bool setContentLength bool @@ -618,7 +618,7 @@ func (r *Request) SetContentLength(l bool) *Request { // // It overrides the credentials set by method [Client.SetBasicAuth]. func (r *Request) SetBasicAuth(username, password string) *Request { - r.UserInfo = &User{Username: username, Password: password} + r.credentials = &credentials{Username: username, Password: password} return r } @@ -677,8 +677,8 @@ func (r *Request) SetDigestAuth(username, password string) *Request { oldTransport := r.client.httpClient.Transport r.client.AddRequestMiddleware(func(c *Client, _ *Request) error { c.httpClient.Transport = &digestTransport{ - digestCredentials: digestCredentials{username, password}, - transport: oldTransport, + credentials: credentials{username, password}, + transport: oldTransport, } return nil }) @@ -1400,8 +1400,8 @@ func (r *Request) Clone(ctx context.Context) *Request { rr.RawPathParams = maps.Clone(r.RawPathParams) // clone basic auth - if r.UserInfo != nil { - rr.UserInfo = r.UserInfo.Clone() + if r.credentials != nil { + rr.credentials = r.credentials.Clone() } // clone cookies diff --git a/request_test.go b/request_test.go index e5db7f60..ba5f5cab 100644 --- a/request_test.go +++ b/request_test.go @@ -220,7 +220,8 @@ func TestPostJSONStructSuccess(t *testing.T) { ts := createPostServer(t) defer ts.Close() - user := &User{Username: "testuser", Password: "testpass"} + user := &credentials{Username: "testuser", Password: "testpass"} + assertEqual(t, "Username: **********, Password: **********", user.String()) c := dcnl().SetJSONEscapeHTML(false) r := c.R(). @@ -246,7 +247,8 @@ func TestPostJSONRPCStructSuccess(t *testing.T) { ts := createPostServer(t) defer ts.Close() - user := &User{Username: "testuser", Password: "testpass"} + user := &credentials{Username: "testuser", Password: "testpass"} + assertEqual(t, "Username: **********, Password: **********", user.String()) c := dcnl().SetJSONEscapeHTML(false) r := c.R(). @@ -276,7 +278,7 @@ func TestPostJSONStructInvalidLogin(t *testing.T) { resp, err := c.R(). SetHeader(hdrContentTypeKey, "application/json; charset=utf-8"). - SetBody(User{Username: "testuser", Password: "testpass1"}). + SetBody(credentials{Username: "testuser", Password: "testpass1"}). SetError(AuthError{}). SetJSONEscapeHTML(false). Post(ts.URL + "/login") @@ -299,7 +301,7 @@ func TestPostJSONErrorRFC7807(t *testing.T) { c := dcnl() resp, err := c.R(). SetHeader(hdrContentTypeKey, "application/json; charset=utf-8"). - SetBody(User{Username: "testuser", Password: "testpass1"}). + SetBody(credentials{Username: "testuser", Password: "testpass1"}). SetError(AuthError{}). Post(ts.URL + "/login?ct=problem") @@ -493,7 +495,7 @@ func TestPostXMLStructSuccess(t *testing.T) { resp, err := dcnldr(). SetHeader(hdrContentTypeKey, "application/xml"). - SetBody(User{Username: "testuser", Password: "testpass"}). + SetBody(credentials{Username: "testuser", Password: "testpass"}). SetContentLength(true). SetResult(&AuthSuccess{}). Post(ts.URL + "/login") @@ -515,7 +517,7 @@ func TestPostXMLStructInvalidLogin(t *testing.T) { resp, err := c.R(). SetHeader(hdrContentTypeKey, "application/xml"). - SetBody(User{Username: "testuser", Password: "testpass1"}). + SetBody(credentials{Username: "testuser", Password: "testpass1"}). Post(ts.URL + "/login") assertError(t, err) @@ -533,7 +535,7 @@ func TestPostXMLStructInvalidResponseXml(t *testing.T) { resp, err := dcnldr(). SetHeader(hdrContentTypeKey, "application/xml"). - SetBody(User{Username: "testuser", Password: "invalidxml"}). + SetBody(credentials{Username: "testuser", Password: "invalidxml"}). SetResult(&AuthSuccess{}). Post(ts.URL + "/login") @@ -617,7 +619,8 @@ func TestRequestInsecureBasicAuth(t *testing.T) { assertError(t, err) assertEqual(t, http.StatusOK, resp.StatusCode()) - assertEqual(t, true, strings.Contains(logBuf.String(), "WARN RESTY Using Basic Auth in HTTP mode is not secure, use HTTPS")) + assertEqual(t, true, strings.Contains(logBuf.String(), + "WARN RESTY Using sensitive credentials in HTTP mode is not secure. Use HTTPS")) t.Logf("Result Success: %q", resp.Result().(*AuthSuccess)) logResponse(t, resp) @@ -1157,7 +1160,7 @@ func TestDetectContentTypeForPointer(t *testing.T) { ts := createPostServer(t) defer ts.Close() - user := &User{Username: "testuser", Password: "testpass"} + user := &credentials{Username: "testuser", Password: "testpass"} resp, err := dcnldr(). SetBody(user). @@ -1987,7 +1990,7 @@ func TestRequestClone(t *testing.T) { // update value of http header - change will only happen on clone clone.SetHeader("X-Header", "clone") // update value of interface type - change will only happen on clone - clone.UserInfo.Username = "clone" + clone.credentials.Username = "clone" clone.bodyBuf.Reset() clone.bodyBuf.WriteString("clone") @@ -2002,8 +2005,8 @@ func TestRequestClone(t *testing.T) { assertEqual(t, "parent", parent.Header.Get("X-Header")) assertEqual(t, "clone", clone.Header.Get("X-Header")) // assert interface type - assertEqual(t, "parent", parent.UserInfo.Username) - assertEqual(t, "clone", clone.UserInfo.Username) + assertEqual(t, "parent", parent.credentials.Username) + assertEqual(t, "clone", clone.credentials.Username) assertEqual(t, "", parent.bodyBuf.String()) assertEqual(t, "clone", clone.bodyBuf.String()) @@ -2017,7 +2020,7 @@ func TestResponseBodyUnlimitedReads(t *testing.T) { ts := createPostServer(t) defer ts.Close() - user := &User{Username: "testuser", Password: "testpass"} + user := &credentials{Username: "testuser", Password: "testpass"} c := dcnl(). SetJSONEscapeHTML(false). @@ -2183,7 +2186,7 @@ func TestRequestSetResultAndSetOutputFile(t *testing.T) { res, err := c.R(). SetHeader(hdrContentTypeKey, "application/json; charset=utf-8"). - SetBody(&User{Username: "testuser", Password: "testpass"}). + SetBody(&credentials{Username: "testuser", Password: "testpass"}). SetResponseBodyUnlimitedReads(true). SetResult(&AuthSuccess{}). SetOutputFile(outputFile). diff --git a/resty.go b/resty.go index b1ce59cf..c05aa5b8 100644 --- a/resty.go +++ b/resty.go @@ -163,6 +163,7 @@ func createClient(hc *http.Client) *Client { queryParams: url.Values{}, formData: url.Values{}, header: http.Header{}, + authScheme: defaultAuthScheme, cookies: make([]*http.Cookie, 0), retryWaitTime: defaultWaitTime, retryMaxWaitTime: defaultMaxWaitTime, diff --git a/resty_test.go b/resty_test.go index 9da4efc4..25fcc65b 100644 --- a/resty_test.go +++ b/resty_test.go @@ -167,7 +167,7 @@ func createGetServer(t *testing.T) *httptest.Server { func handleLoginEndpoint(t *testing.T, w http.ResponseWriter, r *http.Request) { if r.URL.Path == "/login" { - user := &User{} + user := &credentials{} // JSON if isJSONContentType(r.Header.Get(hdrContentTypeKey)) { diff --git a/util.go b/util.go index f4c932e0..942bd9ba 100644 --- a/util.go +++ b/util.go @@ -69,6 +69,23 @@ func (l *logger) output(format string, v ...any) { l.l.Printf(format, v...) } +// credentials type is to hold an username and password information +type credentials struct { + Username, Password string +} + +// Clone method returns clone of c. +func (c *credentials) Clone() *credentials { + cc := new(credentials) + *cc = *c + return cc +} + +// String method returns masked value of username and password +func (c credentials) String() string { + return "Username: **********, Password: **********" +} + //‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾ // Package Helper methods //_______________________________________________________________________