From bde45826d82ff6de2fd6446008eccd19605efdb4 Mon Sep 17 00:00:00 2001 From: Karina Munoz Gonzalez Date: Mon, 4 Dec 2023 09:27:30 -0600 Subject: [PATCH] BCDA-7416: Remove duplicate logging (#148) --- ssas/logger.go | 1 + ssas/service/api_common.go | 1 + ssas/service/main/main.go | 11 +++++------ ssas/service/public/api.go | 19 ++++++------------- ssas/service/server.go | 6 ++++-- 5 files changed, 17 insertions(+), 21 deletions(-) diff --git a/ssas/logger.go b/ssas/logger.go index b4300ab5..36530e62 100644 --- a/ssas/logger.go +++ b/ssas/logger.go @@ -103,6 +103,7 @@ func OperationCalled(data Event) { // OperationFailed should be called after an event's failure, and should always be preceded by // a call to OperationStarted func OperationFailed(data Event) { + // *TODO: refactor. Remove OperationFailed to prevent duplicate logging. Address areas affected by removal. mergeNonEmpty(data).WithField("Event", "OperationFailed").Print(data.Help) } diff --git a/ssas/service/api_common.go b/ssas/service/api_common.go index 0258f834..447254f0 100644 --- a/ssas/service/api_common.go +++ b/ssas/service/api_common.go @@ -26,6 +26,7 @@ func WriteHTTPSError(w http.ResponseWriter, e ssas.ErrorResponse, errorStatus in // Follow RFC 7591 format for input errors func JSONError(w http.ResponseWriter, errorStatus int, statusText string, statusDescription string) { + // *TODO: address duplicate logging. Remove logging from JSONError but make sure areas that rely on it for logging, still have logging after removal. e := ssas.ErrorResponse{Error: statusText, ErrorDescription: statusDescription} WriteHTTPSError(w, e, errorStatus) diff --git a/ssas/service/main/main.go b/ssas/service/main/main.go index 06c5d65c..4208ec1c 100644 --- a/ssas/service/main/main.go +++ b/ssas/service/main/main.go @@ -1,14 +1,13 @@ /* Package main System-to-System Authentication Service - + The System-to-System Authentication Service (SSAS) enables one software system to authenticate and authorize another software system. In this model, the Systems act automatically, independent of a human user identity. Human users are involved only to administer the Service, including establishing the identities and privileges of participating systems. - + For more details see our repository readme and Postman tests: - https://github.com/CMSgov/bcda-ssas-app - https://github.com/CMSgov/bcda-ssas-app/tree/master/test/postman_test - + If you have a Client ID and Secret you can use this page to explore the API. To do this, click the green "Authorize" button below and enter your Client ID and secret in the Basic Authentication username and password boxes. - Until you click logout your token will be presented with every request made. To make requests click on the "Try it out" button for the desired endpoint. Version: 1.0.0 @@ -21,7 +20,7 @@ Until you click logout your token will be presented with every request made. To SecurityDefinitions: basic_auth: type: basic - + swagger:meta */ package main @@ -316,7 +315,7 @@ func newAdminSystem(name string) { func listIPs() { ips, err := ssas.GetAllIPs() if err != nil { - panic("unable to get registered IPs") + ssas.Logger.Fatalf("unable to get registered IPs: %s", err) } listOfIps := strings.Join(ips, "\n") fmt.Fprintln(output, listOfIps) diff --git a/ssas/service/public/api.go b/ssas/service/public/api.go index a001f6e7..5b505e79 100644 --- a/ssas/service/public/api.go +++ b/ssas/service/public/api.go @@ -249,9 +249,10 @@ func token(w http.ResponseWriter, r *http.Request) { service.JSONError(w, http.StatusUnauthorized, http.StatusText(http.StatusUnauthorized), "invalid client id") return } - err = ValidateSecret(system, secret, w, r) + err = ValidateSecret(system, secret, r) if err != nil { ssas.Logger.Error("The client id and secret cannot be validated: ", err.Error()) + service.JSONError(w, http.StatusUnauthorized, http.StatusText(http.StatusUnauthorized), err.Error()) return } @@ -292,24 +293,16 @@ func token(w http.ResponseWriter, r *http.Request) { render.JSON(w, r, m) } -func ValidateSecret(system ssas.System, secret string, w http.ResponseWriter, r *http.Request) (err error) { +func ValidateSecret(system ssas.System, secret string, r *http.Request) (err error) { savedSecret, err := system.GetSecret(r.Context()) - if err != nil { - ssas.Logger.Errorf("Error getting secret: %s", err.Error()) - service.JSONError(w, http.StatusUnauthorized, http.StatusText(http.StatusUnauthorized), "Error getting secret") - return err - } else if !ssas.Hash(savedSecret.Hash).IsHashOf(secret) { - ssas.Logger.Errorf("The incoming client secret is invalid") - service.JSONError(w, http.StatusUnauthorized, http.StatusText(http.StatusUnauthorized), constants.InvalidClientSecret) + if !ssas.Hash(savedSecret.Hash).IsHashOf(secret) { return errors.New(constants.InvalidClientSecret) } if savedSecret.IsExpired() { - ssas.Logger.Error("Credentials were expired") - service.JSONError(w, http.StatusUnauthorized, http.StatusText(http.StatusUnauthorized), "credentials expired") - return errors.New("The saved client secret is expired") + return errors.New("The saved client credendials are expired") } - return nil + return err } func tokenV2(w http.ResponseWriter, r *http.Request) { diff --git a/ssas/service/server.go b/ssas/service/server.go index 8dbfffc6..23af04fa 100644 --- a/ssas/service/server.go +++ b/ssas/service/server.go @@ -52,7 +52,7 @@ type Server struct { func ChooseSigningKey(signingKeyPath, signingKey string) (*rsa.PrivateKey, error) { var key *rsa.PrivateKey = nil var error error = nil - + // *TODO: To prevent duplicate logging, remove error handling out of this function. Return error and log error outside of function. if signingKey == "" && signingKeyPath != "" { sk, err := GetPrivateKey(signingKeyPath) if err != nil { @@ -84,6 +84,7 @@ func ChooseSigningKey(signingKeyPath, signingKey string) (*rsa.PrivateKey, error // NewServer correctly initializes an instance of the Server type. func NewServer(name, port, version string, info interface{}, routes *chi.Mux, notSecure bool, useMTLS bool, signingKey *rsa.PrivateKey, ttl time.Duration, clientAssertAud string) *Server { + if signingKey == nil { ssas.Logger.Error("Private Key is nil") return nil @@ -159,7 +160,8 @@ func (s *Server) LogRoutes() { banner := fmt.Sprintf("Routes for %s at port %s: ", s.name, s.port) routes, err := s.ListRoutes() if err != nil { - ssas.Logger.Infof("%s routing error: %v", banner, err) + ssas.Logger.Errorf("%s routing error: %v", banner, err) + return } ssas.Logger.Infof("%s %v", banner, routes) }