BCDA-7416: Remove duplicate logging (#148)

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)
 }
 

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)

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)

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) {

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)
 }