From ac1402711758a5a0d7ec37adba5427b20366de21 Mon Sep 17 00:00:00 2001 From: pradipdkshirsagar Date: Mon, 22 Jun 2020 13:56:27 +0530 Subject: [PATCH] 1. Updated the implmentation of jwt token with org id. 2. Updated the test cases of reported recognition and moderation recognition. --- go-backend/service/common_http_test.go | 4 +- .../service/recognition_moderation_http.go | 22 ++++-- .../recognition_moderation_http_test.go | 67 ++++++++++++++++ .../service/reported_recognition_http.go | 24 ++++-- .../service/reported_recognition_http_test.go | 69 ++++++++++++++++ go-backend/service/response.go | 2 +- go-backend/service/session_http.go | 78 +++++++++++++------ 7 files changed, 225 insertions(+), 41 deletions(-) diff --git a/go-backend/service/common_http_test.go b/go-backend/service/common_http_test.go index 47bc1bd66..07d068a9d 100644 --- a/go-backend/service/common_http_test.go +++ b/go-backend/service/common_http_test.go @@ -46,11 +46,11 @@ func makeHTTPCall(method, path, requestURL, body string, handlerFunc http.Handle // requestURL: current request path (eg: /users/1) func makeHTTPCallWithJWTMiddleware(method, path, requestURL, body string, handlerFunc http.HandlerFunc) (recorder *httptest.ResponseRecorder) { // create jwt token with userID - JWTToken, _ := newJWT(1) + JWTToken, _ := newJWT(1, 1) // create a http request using the given parameters req, _ := http.NewRequest(method, requestURL, strings.NewReader(body)) - req.Header.Set("Authorization", "Bearer " + JWTToken) + req.Header.Set("Authorization", "Bearer "+JWTToken) // test recorder created for capturing api responses recorder = httptest.NewRecorder() diff --git a/go-backend/service/recognition_moderation_http.go b/go-backend/service/recognition_moderation_http.go index a01ddce98..8a16fdf81 100644 --- a/go-backend/service/recognition_moderation_http.go +++ b/go-backend/service/recognition_moderation_http.go @@ -5,7 +5,6 @@ import ( "net/http" "strconv" - jwt "github.com/dgrijalva/jwt-go" "github.com/gorilla/mux" logger "github.com/sirupsen/logrus" ae "joshsoftware/peerly/apperrors" @@ -14,13 +13,20 @@ import ( func createRecognitionModerationHandler(deps Dependencies) http.HandlerFunc { return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { - parsedToken := req.Context().Value("user").(*jwt.Token) - claims := parsedToken.Claims.(jwt.MapClaims) - - userID, err := strconv.Atoi(claims["sub"].(string)) + ctx := req.Context() + userID, _, err := validateJWTToken(ctx, deps.Store) + if err == ae.ErrInvalidToken { + logger.WithField("err", err.Error()).Error("Invalid user organization with organization domain") + repsonse(rw, http.StatusUnauthorized, errorResponse{ + Error: messageObject{Message: err.Error()}, + }) + return + } if err != nil { - logger.Error(ae.ErrJSONParseFail, "Error parsing JSON for token response", err) - ae.JSONError(rw, http.StatusInternalServerError, err) + logger.WithField("err", err.Error()).Error("Error while validating the jwt token") + repsonse(rw, http.StatusInternalServerError, errorResponse{ + Error: messageObject{Message: err.Error()}, + }) return } @@ -57,7 +63,7 @@ func createRecognitionModerationHandler(deps Dependencies) http.HandlerFunc { return } - resp, err := deps.Store.CreateRecognitionModeration(req.Context(), recognitionID, recognitionModeration) + resp, err := deps.Store.CreateRecognitionModeration(ctx, recognitionID, recognitionModeration) if err != nil { logger.WithField("err", err.Error()).Error("Error while creating recognition moderation") repsonse(rw, http.StatusInternalServerError, errorResponse{ diff --git a/go-backend/service/recognition_moderation_http_test.go b/go-backend/service/recognition_moderation_http_test.go index 6f4da7137..32baef6ca 100644 --- a/go-backend/service/recognition_moderation_http_test.go +++ b/go-backend/service/recognition_moderation_http_test.go @@ -22,7 +22,29 @@ func (suite *RecognitionModerationHandlerTestSuite) SetupTest() { suite.dbMock = &db.DBMockStore{} } +func (suite *RecognitionModerationHandlerTestSuite) setupJWTTokenTest(valid bool) { + var orgID int + if valid { + orgID = 1 + } else { + orgID = 2 + } + suite.dbMock.On("GetUser", mock.Anything, mock.Anything).Return( + db.User{ + ID: 1, + OrgID: orgID, + Name: "test2", + Email: "test@gmail.com", + DisplayName: "test", + ProfileImageURL: "test.jpg", + RoleID: 10, + Hi5QuotaBalance: 5, + }, nil, + ) +} + func (suite *RecognitionModerationHandlerTestSuite) TestCreateRecognitionModerationSuccess() { + suite.setupJWTTokenTest(true) now := time.Now().Unix() isInappropriate := false suite.dbMock.On("CreateRecognitionModeration", mock.Anything, mock.Anything, mock.Anything).Return(db.RecognitionModeration{ @@ -54,6 +76,7 @@ func (suite *RecognitionModerationHandlerTestSuite) TestCreateRecognitionModerat } func (suite *RecognitionModerationHandlerTestSuite) TestCreateRecognitionModerationWhenInvalidJSONFormat() { + suite.setupJWTTokenTest(true) body := `{ "is_inappropriate": false "comment": "Comment Test" @@ -73,6 +96,7 @@ func (suite *RecognitionModerationHandlerTestSuite) TestCreateRecognitionModerat } func (suite *RecognitionModerationHandlerTestSuite) TestCreateRecognitionModerationWhenEmptyIsInappropriate() { + suite.setupJWTTokenTest(true) body := `{ "comment": "Comment Test" }` @@ -91,6 +115,7 @@ func (suite *RecognitionModerationHandlerTestSuite) TestCreateRecognitionModerat } func (suite *RecognitionModerationHandlerTestSuite) TestCreateRecognitionModerationWhenDBFailure() { + suite.setupJWTTokenTest(true) suite.dbMock.On("CreateRecognitionModeration", mock.Anything, mock.Anything, mock.Anything).Return(db.RecognitionModeration{}, errors.New("error creating reported recognition")) body := `{ @@ -110,3 +135,45 @@ func (suite *RecognitionModerationHandlerTestSuite) TestCreateRecognitionModerat assert.Equal(suite.T(), `{"error":{"message":"Internal server error"}}`, recorder.Body.String()) suite.dbMock.AssertExpectations(suite.T()) } + +func (suite *RecognitionModerationHandlerTestSuite) TestCreateRecognitionModerationWhenInvalidTokenWithOrganizationMismatch() { + suite.setupJWTTokenTest(false) + + body := `{ + "is_inappropriate": false, + "reason": "Comment Test" + }` + + recorder := makeHTTPCallWithJWTMiddleware( + http.MethodPost, + "/recognitions/{recognition_id:[0-9]+}/review", + "/recognitions/1/review", + body, + createRecognitionModerationHandler(Dependencies{Store: suite.dbMock}), + ) + + assert.Equal(suite.T(), http.StatusUnauthorized, recorder.Code) + assert.Equal(suite.T(), `{"error":{"message":"Invalid Token"}}`, recorder.Body.String()) + suite.dbMock.AssertExpectations(suite.T()) +} + +func (suite *RecognitionModerationHandlerTestSuite) TestCreateRecognitionModerationWhenDBFailureForValidateToken() { + suite.dbMock.On("GetUser", mock.Anything, mock.Anything).Return(db.User{}, errors.New("Internal server error")) + + body := `{ + "is_inappropriate": false, + "reason": "Comment Test" + }` + + recorder := makeHTTPCallWithJWTMiddleware( + http.MethodPost, + "/recognitions/{recognition_id:[0-9]+}/review", + "/recognitions/1/review", + body, + createRecognitionModerationHandler(Dependencies{Store: suite.dbMock}), + ) + + assert.Equal(suite.T(), http.StatusInternalServerError, recorder.Code) + assert.Equal(suite.T(), `{"error":{"message":"Internal server error"}}`, recorder.Body.String()) + suite.dbMock.AssertExpectations(suite.T()) +} diff --git a/go-backend/service/reported_recognition_http.go b/go-backend/service/reported_recognition_http.go index d7124cee3..6b6c8fe4f 100644 --- a/go-backend/service/reported_recognition_http.go +++ b/go-backend/service/reported_recognition_http.go @@ -5,7 +5,6 @@ import ( "net/http" "strconv" - jwt "github.com/dgrijalva/jwt-go" "github.com/gorilla/mux" logger "github.com/sirupsen/logrus" ae "joshsoftware/peerly/apperrors" @@ -14,13 +13,22 @@ import ( func createReportedRecognitionHandler(deps Dependencies) http.HandlerFunc { return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { - parsedToken := req.Context().Value("user").(*jwt.Token) - claims := parsedToken.Claims.(jwt.MapClaims) - - userID, err := strconv.Atoi(claims["sub"].(string)) + ctx := req.Context() + userID, _, err := validateJWTToken(ctx, deps.Store) + if err == ae.ErrInvalidToken { + logger.WithField("err", err.Error()).Error("Invalid user organization with organization domain") + repsonse(rw, http.StatusUnauthorized, errorResponse{ + Error: messageObject{Message: err.Error()}, + }) + return + } if err != nil { - logger.Error(ae.ErrJSONParseFail, "Error parsing JSON for token response", err) - ae.JSONError(rw, http.StatusInternalServerError, err) + logger.WithField("err", err.Error()).Error("Error while validating the jwt token") + repsonse(rw, http.StatusInternalServerError, errorResponse{ + Error: messageObject{ + Message: "Internal server error", + }, + }) return } @@ -57,7 +65,7 @@ func createReportedRecognitionHandler(deps Dependencies) http.HandlerFunc { return } - resp, err := deps.Store.CreateReportedRecognition(req.Context(), recognitionID, reportedRecognition) + resp, err := deps.Store.CreateReportedRecognition(ctx, recognitionID, reportedRecognition) if err != nil { logger.WithField("err", err.Error()).Error("Error while creating reported recognition") repsonse(rw, http.StatusInternalServerError, errorResponse{ diff --git a/go-backend/service/reported_recognition_http_test.go b/go-backend/service/reported_recognition_http_test.go index 714cdf9be..c2821b2a6 100644 --- a/go-backend/service/reported_recognition_http_test.go +++ b/go-backend/service/reported_recognition_http_test.go @@ -22,7 +22,29 @@ func (suite *ReportedRecognitionHandlerTestSuite) SetupTest() { suite.dbMock = &db.DBMockStore{} } +func (suite *ReportedRecognitionHandlerTestSuite) setupJWTTokenTest(valid bool) { + var orgID int + if valid { + orgID = 1 + } else { + orgID = 2 + } + suite.dbMock.On("GetUser", mock.Anything, mock.Anything).Return( + db.User{ + ID: 1, + OrgID: orgID, + Name: "test2", + Email: "test@gmail.com", + DisplayName: "test", + ProfileImageURL: "test.jpg", + RoleID: 10, + Hi5QuotaBalance: 5, + }, nil, + ) +} + func (suite *ReportedRecognitionHandlerTestSuite) TestCreateReportedRecognitionSuccess() { + suite.setupJWTTokenTest(true) now := time.Now().Unix() suite.dbMock.On("CreateReportedRecognition", mock.Anything, mock.Anything, mock.Anything).Return(db.ReportedRecognition{ ID: 1, @@ -53,6 +75,7 @@ func (suite *ReportedRecognitionHandlerTestSuite) TestCreateReportedRecognitionS } func (suite *ReportedRecognitionHandlerTestSuite) TestCreateReportedRecognitionWhenInvalidJSONFormat() { + suite.setupJWTTokenTest(true) body := `{ "mark_as": "fraud" "reason": "Reason Test" @@ -72,6 +95,7 @@ func (suite *ReportedRecognitionHandlerTestSuite) TestCreateReportedRecognitionW } func (suite *ReportedRecognitionHandlerTestSuite) TestCreateReportedRecognitionWhenEmptyReportedRecognitionType() { + suite.setupJWTTokenTest(true) body := `{ "mark_as": "", "reason": "Reason Test" @@ -91,6 +115,7 @@ func (suite *ReportedRecognitionHandlerTestSuite) TestCreateReportedRecognitionW } func (suite *ReportedRecognitionHandlerTestSuite) TestCreateReportedRecognitionWhenInvalidReportedRecognitionType() { + suite.setupJWTTokenTest(true) body := `{ "mark_as": "XYZ", "reason": "Reason Test" @@ -110,6 +135,7 @@ func (suite *ReportedRecognitionHandlerTestSuite) TestCreateReportedRecognitionW } func (suite *ReportedRecognitionHandlerTestSuite) TestCreateReportedRecognitionWhenEmptyReasonForReport() { + suite.setupJWTTokenTest(true) body := `{ "mark_as": "fraud", "reason": "" @@ -129,6 +155,7 @@ func (suite *ReportedRecognitionHandlerTestSuite) TestCreateReportedRecognitionW } func (suite *ReportedRecognitionHandlerTestSuite) TestCreateReportedRecognitionWhenDBFailure() { + suite.setupJWTTokenTest(true) suite.dbMock.On("CreateReportedRecognition", mock.Anything, mock.Anything, mock.Anything).Return(db.ReportedRecognition{}, errors.New("error creating reported recognition")) body := `{ @@ -148,3 +175,45 @@ func (suite *ReportedRecognitionHandlerTestSuite) TestCreateReportedRecognitionW assert.Equal(suite.T(), `{"error":{"message":"Internal server error"}}`, recorder.Body.String()) suite.dbMock.AssertExpectations(suite.T()) } + +func (suite *ReportedRecognitionHandlerTestSuite) TestCreateReportedRecognitionWhenInvalidTokenWithOrganizationMismatch() { + suite.setupJWTTokenTest(false) + + body := `{ + "mark_as": "fraud", + "reason": "Reason Test" + }` + + recorder := makeHTTPCallWithJWTMiddleware( + http.MethodPost, + "/recognitions/{recognition_id:[0-9]+}/report", + "/recognitions/1/report", + body, + createReportedRecognitionHandler(Dependencies{Store: suite.dbMock}), + ) + + assert.Equal(suite.T(), http.StatusUnauthorized, recorder.Code) + assert.Equal(suite.T(), `{"error":{"message":"Invalid Token"}}`, recorder.Body.String()) + suite.dbMock.AssertExpectations(suite.T()) +} + +func (suite *ReportedRecognitionHandlerTestSuite) TestCreateReportedRecognitionWhenDBFailureForValidateToken() { + suite.dbMock.On("GetUser", mock.Anything, mock.Anything).Return(db.User{}, errors.New("Internal server error")) + + body := `{ + "mark_as": "fraud", + "reason": "Reason Test" + }` + + recorder := makeHTTPCallWithJWTMiddleware( + http.MethodPost, + "/recognitions/{recognition_id:[0-9]+}/report", + "/recognitions/1/report", + body, + createReportedRecognitionHandler(Dependencies{Store: suite.dbMock}), + ) + + assert.Equal(suite.T(), http.StatusInternalServerError, recorder.Code) + assert.Equal(suite.T(), `{"error":{"message":"Internal server error"}}`, recorder.Body.String()) + suite.dbMock.AssertExpectations(suite.T()) +} diff --git a/go-backend/service/response.go b/go-backend/service/response.go index 541d80b7a..a8f7aa0d8 100644 --- a/go-backend/service/response.go +++ b/go-backend/service/response.go @@ -28,7 +28,7 @@ type errorObject struct { func repsonse(rw http.ResponseWriter, status int, responseBody interface{}) { respBytes, err := json.Marshal(responseBody) if err != nil { - logger.WithField("err", err.Error()).Error("Error while marshaling core values data") + logger.WithField("err", err.Error()).Error("Error while marshaling response data") rw.WriteHeader(http.StatusInternalServerError) return } diff --git a/go-backend/service/session_http.go b/go-backend/service/session_http.go index 8d082f4fd..c446256db 100644 --- a/go-backend/service/session_http.go +++ b/go-backend/service/session_http.go @@ -1,18 +1,20 @@ package service import ( + "context" "encoding/json" "io/ioutil" - ae "joshsoftware/peerly/apperrors" - "joshsoftware/peerly/config" - "joshsoftware/peerly/db" - log "joshsoftware/peerly/util/log" "net/http" "net/url" "strconv" "time" jwt "github.com/dgrijalva/jwt-go" + logger "github.com/sirupsen/logrus" + ae "joshsoftware/peerly/apperrors" + "joshsoftware/peerly/config" + "joshsoftware/peerly/db" + log "joshsoftware/peerly/util/log" ) // OAuthUser - a struct that represents the "user" we'll get back from Google's /userinfo query @@ -133,6 +135,17 @@ func handleAuth(deps Dependencies) http.HandlerFunc { return } + // Check the OAuth User's domain and see if it's already in our database + // TODO - We need a way to test this both programmatically and by hand. + // That necessitates a Google account associated w/ a domain that isn't Josh Software + org, err := deps.Store.GetOrganizationByDomainName(ctx, user.Domain) + if err != nil { + // Log error, push out a JSON response, and halt authentication + log.Error(ae.ErrDomainNotRegistered(user.Email), ("Domain: " + user.Domain + " Email " + user.Email), err) + ae.JSONError(rw, http.StatusForbidden, err) + return + } + // See if there's an existing user that matches the oAuth user existingUser, err := deps.Store.GetUserByEmail(ctx, user.Email) if err != nil && err != ae.ErrRecordNotFound { @@ -142,17 +155,6 @@ func handleAuth(deps Dependencies) http.HandlerFunc { } if err == ae.ErrRecordNotFound { - // Check the OAuth User's domain and see if it's already in our database - // TODO - We need a way to test this both programmatically and by hand. - // That necessitates a Google account associated w/ a domain that isn't Josh Software - org, err := deps.Store.GetOrganizationByDomainName(ctx, user.Domain) - if err != nil { - // Log error, push out a JSON response, and halt authentication - log.Error(ae.ErrDomainNotRegistered(user.Email), ("Domain: " + user.Domain + " Email " + user.Email), err) - ae.JSONError(rw, http.StatusForbidden, err) - return - } - // Organization DOES exist in the database. Create the user. existingUser, err = deps.Store.CreateNewUser(ctx, db.User{ Email: user.Email, @@ -168,7 +170,7 @@ func handleAuth(deps Dependencies) http.HandlerFunc { // By the time we get here, we definitely have an existingUser object. // Looks like a valid user authenticated by Google. User's org is in our orgs table. Issue a JWT. - authToken, err := newJWT(existingUser.ID) + authToken, err := newJWT(existingUser.ID, org.ID) if err != nil { log.Error(ae.ErrUnknown, "Unknown/unexpected error while creating JWT for "+existingUser.Email, err) ae.JSONError(rw, http.StatusInternalServerError, err) @@ -193,7 +195,7 @@ func handleAuth(deps Dependencies) http.HandlerFunc { // newJWT() - Creates and returns a new JSON Web Token to be sent to an API consumer on valid // authentication, so they can re-use it by sending it in the Authorization header on subsequent // requests. -func newJWT(userID int) (newToken string, err error) { +func newJWT(userID, organizationID int) (newToken string, err error) { signingKey := config.JWTKey() if signingKey == nil { log.Error(ae.ErrNoSigningKey, "Application error: No signing key configured", err) @@ -201,11 +203,12 @@ func newJWT(userID int) (newToken string, err error) { } expiryTime := time.Now().Add(time.Duration(config.JWTExpiryDurationHours()) * time.Hour).Unix() - claims := &jwt.StandardClaims{ - ExpiresAt: expiryTime, - Issuer: "joshsoftware.com", - IssuedAt: time.Now().Unix(), - Subject: strconv.Itoa(userID), + claims := &jwt.MapClaims{ + "exp": expiryTime, + "iss": "joshsoftware.com", + "iat": time.Now().Unix(), + "sub": strconv.Itoa(userID), + "org": strconv.Itoa(organizationID), } token := jwt.NewWithClaims(jwt.SigningMethodHS256, claims) @@ -249,3 +252,34 @@ func handleLogout(deps Dependencies) http.HandlerFunc { return }) } + +// validateJWTToken() - validates and returns a user id, orgnization id and error. +func validateJWTToken(ctx context.Context, storer db.Storer) (userID, orgID int, err error) { + parsedToken := ctx.Value("user").(*jwt.Token) + claims := parsedToken.Claims.(jwt.MapClaims) + + userID, err = strconv.Atoi(claims["sub"].(string)) + if err != nil { + logger.Error(ae.ErrJSONParseFail, "Error parsing JSON for token response", err) + return + } + + orgID, err = strconv.Atoi(claims["org"].(string)) + if err != nil { + logger.Error(ae.ErrJSONParseFail, "Error parsing JSON for token response", err) + return + } + + currentUser, err := storer.GetUser(ctx, userID) + if err != nil { + logger.WithField("err", err.Error()).Error("Error while fetching User") + return + } + + if currentUser.OrgID != orgID { + err = ae.ErrInvalidToken + logger.WithField("err", err.Error()).Error("Mismatch with user organization and current organization") + } + + return +}