From 4913ee3f32fa60bd59dccc9a3020c5fb8bd07df4 Mon Sep 17 00:00:00 2001
From: Sergey Yakubov <sergey.yakubov@desy.de>
Date: Fri, 26 Nov 2021 11:42:55 +0100
Subject: [PATCH] fix golang logs, auth error type

---
 .../src/asapo_authorizer/server/authorize.go   |  2 +-
 .../asapo_authorizer/server/authorize_test.go  | 18 ++++++++++++++++++
 .../src/asapo_broker/server/authorizer_test.go |  2 +-
 .../server/process_request_test.go             |  4 ++--
 .../src/asapo_broker/server/request_common.go  | 11 +++++------
 .../src/asapo_common/logger/logrus_logger.go   |  8 ++++++--
 .../go/src/asapo_common/logger/logrus_test.go  | 10 ++++++----
 .../go/src/asapo_common/utils/authorization.go |  2 +-
 .../asapo_common/utils/authorization_test.go   |  4 ++--
 .../request_handler/authorization_client.cpp   | 10 ++++++----
 10 files changed, 48 insertions(+), 23 deletions(-)

diff --git a/authorizer/src/asapo_authorizer/server/authorize.go b/authorizer/src/asapo_authorizer/server/authorize.go
index 1b984a9c4..295a29ed2 100644
--- a/authorizer/src/asapo_authorizer/server/authorize.go
+++ b/authorizer/src/asapo_authorizer/server/authorize.go
@@ -372,7 +372,7 @@ func routeAuthorize(w http.ResponseWriter, r *http.Request) {
 	}
 
 	w.WriteHeader(http.StatusOK)
-	w.Write([]byte(res))
+	w.Write(res)
 }
 
 func checkRole(w http.ResponseWriter, r *http.Request, role string) error {
diff --git a/authorizer/src/asapo_authorizer/server/authorize_test.go b/authorizer/src/asapo_authorizer/server/authorize_test.go
index 1c6e64005..e3bb6c11e 100644
--- a/authorizer/src/asapo_authorizer/server/authorize_test.go
+++ b/authorizer/src/asapo_authorizer/server/authorize_test.go
@@ -387,3 +387,21 @@ func TestGetBeamtimeInfo(t *testing.T) {
 	}
 
 }
+
+func TestExpiredToken(t *testing.T) {
+	Auth = authorization.NewAuth(utils.NewJWTAuth("secret_user"), utils.NewJWTAuth("secret_admin"), utils.NewJWTAuth("secret"))
+	token := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJleHAiOjE2MzU3NTMxMDksImp0aSI6ImMyOTR0NWFodHY1am9vZHVoaGNnIiwic3ViIjoiYnRfMTEwMTIxNzEiLCJFeHRyYUNsYWltcyI6eyJBY2Nlc3NUeXBlcyI6WyJyZWFkIiwid3JpdGUiXX19.kITePbv_dXY2ACxpAQ-PeQJPQtnR02bMoFrXq0Pbcm0"
+	request := authorizationRequest{"asapo_test%%"+token, "host"}
+	creds, _ := getSourceCredentials(request)
+
+	creds.Token = token
+	creds.DataSource = "test"
+	creds.BeamtimeId = "11012171"
+	creds.Beamline = "p21.2"
+	_, err := authorizeByToken(creds)
+	assert.Error(t, err, "")
+	if (err!=nil) {
+		assert.Contains(t, err.Error(), "expired")
+	}
+
+}
diff --git a/broker/src/asapo_broker/server/authorizer_test.go b/broker/src/asapo_broker/server/authorizer_test.go
index a58681460..f854b2246 100644
--- a/broker/src/asapo_broker/server/authorizer_test.go
+++ b/broker/src/asapo_broker/server/authorizer_test.go
@@ -47,7 +47,7 @@ func responseOk() (*http.Response, error) {
 }
 
 func responseUnauth() (*http.Response, error) {
-	r := ioutil.NopCloser(bytes.NewReader([]byte("wrong JWT token")))
+	r := ioutil.NopCloser(bytes.NewReader([]byte("wrong or expired JWT token")))
 	return &http.Response{
 		StatusCode: http.StatusUnauthorized,
 		Body:       r,
diff --git a/broker/src/asapo_broker/server/process_request_test.go b/broker/src/asapo_broker/server/process_request_test.go
index f62d12814..a9c5f53f3 100644
--- a/broker/src/asapo_broker/server/process_request_test.go
+++ b/broker/src/asapo_broker/server/process_request_test.go
@@ -45,7 +45,7 @@ func (a *MockAuthServer) AuthorizeToken(tokenJWT string) (token Token, err error
 		}, nil
 	}
 
-	return Token{}, AuthorizationError{errors.New("wrong JWT token"),http.StatusUnauthorized}
+	return Token{}, &AuthorizationError{errors.New("wrong or expired JWT token"),http.StatusUnauthorized}
 }
 
 func prepareTestAuth() {
@@ -148,7 +148,7 @@ func TestProcessRequestTestSuite(t *testing.T) {
 
 func (suite *ProcessRequestTestSuite) TestProcessRequestWithWrongToken() {
 
-	logger.MockLog.On("WithFields", mock.MatchedBy(containsMatcherMap("wrong JWT token")))
+	logger.MockLog.On("WithFields", mock.MatchedBy(containsMatcherMap("wrong or expired JWT token")))
 	logger.MockLog.On("Error", mock.MatchedBy(containsMatcherStr("cannot authorize request")))
 
 	w := doRequest("/beamtime/" + expectedBeamtimeId + "/" + expectedSource + "/" + expectedStream + "/" + expectedGroupID + "/next" + suffixWithWrongToken)
diff --git a/broker/src/asapo_broker/server/request_common.go b/broker/src/asapo_broker/server/request_common.go
index 42c34e4a0..cda3a0995 100644
--- a/broker/src/asapo_broker/server/request_common.go
+++ b/broker/src/asapo_broker/server/request_common.go
@@ -10,9 +10,8 @@ import (
 
 func writeAuthAnswer(w http.ResponseWriter, requestOp string, db_name string, err error) {
 	logger.WithFields(map[string]interface{}{"operation": requestOp, "cause": err.Error()}).Error("cannot authorize request")
-
 	switch er := err.(type) {
-	case AuthorizationError:
+	case *AuthorizationError:
 		w.WriteHeader(er.statusCode)
 	default:
 		w.WriteHeader(http.StatusServiceUnavailable)
@@ -53,7 +52,7 @@ func authorize(r *http.Request, beamtime_id string, needWriteAccess bool) error
 	tokenJWT := r.URL.Query().Get("token")
 
 	if len(tokenJWT) == 0 {
-		return AuthorizationError{errors.New("cannot extract token from request"), http.StatusBadRequest}
+		return &AuthorizationError{errors.New("cannot extract token from request"), http.StatusBadRequest}
 	}
 
 	token, err := auth.AuthorizeToken(tokenJWT)
@@ -71,18 +70,18 @@ func authorize(r *http.Request, beamtime_id string, needWriteAccess bool) error
 
 func checkSubject(subject string, beamtime_id string) error {
 	if subject != utils.SubjectFromBeamtime(beamtime_id) {
-		return AuthorizationError{errors.New("wrong token subject"), http.StatusUnauthorized}
+		return &AuthorizationError{errors.New("wrong token subject"), http.StatusUnauthorized}
 	}
 	return nil
 }
 
 func checkAccessType(accessTypes []string, needWriteAccess bool) error {
 	if needWriteAccess && !utils.StringInSlice("write", accessTypes) {
-		return AuthorizationError{errors.New("wrong token access type"), http.StatusUnauthorized}
+		return &AuthorizationError{errors.New("wrong token access type"), http.StatusUnauthorized}
 	}
 
 	if !utils.StringInSlice("read", accessTypes) {
-		return AuthorizationError{errors.New("wrong token access type"), http.StatusUnauthorized}
+		return &AuthorizationError{errors.New("wrong token access type"), http.StatusUnauthorized}
 	}
 	return nil
 }
diff --git a/common/go/src/asapo_common/logger/logrus_logger.go b/common/go/src/asapo_common/logger/logrus_logger.go
index d41432252..88b411143 100644
--- a/common/go/src/asapo_common/logger/logrus_logger.go
+++ b/common/go/src/asapo_common/logger/logrus_logger.go
@@ -15,8 +15,11 @@ func (l *logRusLogger) SetSource(source string) {
 
 
 func (l *logRusLogger) WithFields(args map[string]interface{}) Logger {
-	l.logger_entry = l.entry().WithFields(args)
-	return l
+	new_log:= &logRusLogger{
+		logger_entry: l.entry().WithFields(args),
+		source:       l.source,
+	}
+	return new_log
 }
 
 
@@ -25,6 +28,7 @@ func (l *logRusLogger) entry() *log.Entry {
 		return l.logger_entry
 	}
 
+
 	formatter := &log.JSONFormatter{
 		FieldMap: log.FieldMap{
 			log.FieldKeyMsg: "message",
diff --git a/common/go/src/asapo_common/logger/logrus_test.go b/common/go/src/asapo_common/logger/logrus_test.go
index 0587dff3c..d6997f888 100644
--- a/common/go/src/asapo_common/logger/logrus_test.go
+++ b/common/go/src/asapo_common/logger/logrus_test.go
@@ -1,7 +1,6 @@
 package logger
 
 import (
-	"fmt"
 	"github.com/sirupsen/logrus/hooks/test"
 	"github.com/stretchr/testify/assert"
 	"testing"
@@ -19,8 +18,11 @@ func logStr(hook *test.Hook) string {
 func TestLog(t *testing.T) {
 	l := &logRusLogger{}
 	hook := test.NewLocal(l.entry().Logger)
-	l.WithFields(map[string]interface{}{"testmap":1}).Info("aaa")
-	fmt.Println(logStr(hook))
-	assert.Contains(t, logStr(hook),"testmap")
+	l.WithFields(map[string]interface{}{"testmap1":1}).Info("aaa")
+	assert.Contains(t, logStr(hook),"testmap1")
+
+	hook.Reset()
+	l.WithFields(map[string]interface{}{"testmap2":1}).Info("bbb")
+	assert.NotContains(t, logStr(hook),"testmap1")
 
 }
diff --git a/common/go/src/asapo_common/utils/authorization.go b/common/go/src/asapo_common/utils/authorization.go
index d707819b9..8a1b11bb2 100644
--- a/common/go/src/asapo_common/utils/authorization.go
+++ b/common/go/src/asapo_common/utils/authorization.go
@@ -151,7 +151,7 @@ func (a *JWTAuth) CheckAndGetContent(token string, extraClaims interface{}, payl
 	// payload ignored
 	c, ok := CheckJWTToken(token,a.Key)
 	if !ok {
-		return nil,errors.New("wrong JWT token")
+		return nil,errors.New("wrong or expired JWT token")
 	}
 	claim,ok  := c.(*CustomClaims)
 	if !ok {
diff --git a/common/go/src/asapo_common/utils/authorization_test.go b/common/go/src/asapo_common/utils/authorization_test.go
index cda7f43b0..b0add6be7 100644
--- a/common/go/src/asapo_common/utils/authorization_test.go
+++ b/common/go/src/asapo_common/utils/authorization_test.go
@@ -1,11 +1,11 @@
 package utils
 
 import (
+	"github.com/stretchr/testify/assert"
 	"net/http"
-	"testing"
 	"net/http/httptest"
+	"testing"
 	"time"
-	"github.com/stretchr/testify/assert"
 )
 
 type authorizationResponse struct {
diff --git a/receiver/src/request_handler/authorization_client.cpp b/receiver/src/request_handler/authorization_client.cpp
index cb5c737f4..2a4ab650e 100644
--- a/receiver/src/request_handler/authorization_client.cpp
+++ b/receiver/src/request_handler/authorization_client.cpp
@@ -22,15 +22,16 @@ Error ErrorFromAuthorizationServerResponse(Error err, const std::string response
     if (err) {
         return_err = asapo::ReceiverErrorTemplates::kInternalServerError.Generate(
             "cannot authorize request");
+        return_err->AddDetails("response", response);
         return_err->SetCause(std::move(err));
     } else {
         if (code != HttpCode::Unauthorized) {
             return_err = asapo::ReceiverErrorTemplates::kInternalServerError.Generate();
-            return_err->AddDetails("response", response)->AddDetails("errorCode", std::to_string(int(
-                code)));
         } else {
             return_err = asapo::ReceiverErrorTemplates::kAuthorizationFailure.Generate();
         }
+        return_err->AddDetails("response", response)->AddDetails("errorCode", std::to_string(int(
+            code)));
     }
     return return_err;
 }
@@ -56,6 +57,7 @@ Error ParseServerResponse(const std::string &response,
                           std::vector<std::string> *access_types,
                           AuthorizationData *data) {
     Error err;
+
     AuthorizationData creds;
     JsonStringParser parser{response};
     std::string stype;
@@ -69,7 +71,7 @@ Error ParseServerResponse(const std::string &response,
         (err = GetSourceTypeFromString(stype, &data->source_type)) ||
         (err = parser.GetString("beamline", &data->beamline));
     if (err) {
-        return ErrorFromAuthorizationServerResponse(std::move(err), "", code);
+        return ErrorFromAuthorizationServerResponse(std::move(err), response, code);
     }
     return nullptr;
 }
@@ -82,7 +84,7 @@ Error UpdateDataFromServerResponse(const std::string &response, HttpCode code, A
     err = ParseServerResponse(response, code, &access_types, data);
     if (err) {
         *data = old_data;
-        return ErrorFromAuthorizationServerResponse(std::move(err), response, code);
+        return err;
     }
 
     err = CheckAccessType(data->source_type, access_types);
-- 
GitLab