Skip to content

Commit a801e3d

Browse files
SankeerthSai Sankeerth
Sankeerth
and
Sai Sankeerth
authored
fix: invalid error response handling during oauth refresh flow (#5401)
* fix: invalid error response handling during oauth refresh flow * chore: adding more tests to increase coverage --------- Co-authored-by: Sai Sankeerth <[email protected]>
1 parent d29eb14 commit a801e3d

File tree

3 files changed

+179
-14
lines changed

3 files changed

+179
-14
lines changed

‎services/oauth/v2/common/constants.go

+5-4
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,11 @@ const (
55
// CategoryAuthStatusInactive Identifier to be sent from destination(during transformation/delivery)
66
CategoryAuthStatusInactive = "AUTH_STATUS_INACTIVE"
77
// RefTokenInvalidGrant Identifier for invalid_grant or access_denied errors(during refreshing the token)
8-
RefTokenInvalidGrant = "ref_token_invalid_grant"
9-
TimeOutError = "timeout"
10-
NetworkError = "network_error"
11-
None = "none"
8+
RefTokenInvalidGrant = "ref_token_invalid_grant"
9+
RefTokenInvalidResponse = "INVALID_REFRESH_RESPONSE"
10+
TimeOutError = "timeout"
11+
NetworkError = "network_error"
12+
None = "none"
1213

1314
DestKey ContextKey = "destination"
1415
SecretKey ContextKey = "secret"

‎services/oauth/v2/oauth.go

+21-10
Original file line numberDiff line numberDiff line change
@@ -330,17 +330,28 @@ func (h *OAuthHandler) GetRefreshTokenErrResp(response string, accountSecret *Ac
330330
h.Logger.Debugn("Failed with error response", logger.NewErrorField(err))
331331
message = fmt.Sprintf("Unmarshal of response unsuccessful: %v", response)
332332
errorType = "unmarshallableResponse"
333-
} else if gjson.Get(response, "body.code").String() == common.RefTokenInvalidGrant {
334-
// User (or) AccessToken (or) RefreshToken has been revoked
335-
bodyMsg := gjson.Get(response, "body.message").String()
336-
if bodyMsg == "" {
337-
// Default message
338-
h.Logger.Debugn("Unable to get body.message", logger.NewStringField("Response", response))
339-
message = ErrPermissionOrTokenRevoked.Error()
340-
} else {
341-
message = bodyMsg
333+
} else {
334+
code := gjson.Get(response, "body.code").String()
335+
switch code {
336+
case common.RefTokenInvalidGrant:
337+
// User (or) AccessToken (or) RefreshToken has been revoked
338+
bodyMsg := gjson.Get(response, "body.message").String()
339+
if bodyMsg == "" {
340+
// Default message
341+
h.Logger.Debugn("Unable to get body.message", logger.NewStringField("Response", response))
342+
message = ErrPermissionOrTokenRevoked.Error()
343+
} else {
344+
message = bodyMsg
345+
}
346+
errorType = common.RefTokenInvalidGrant
347+
case common.RefTokenInvalidResponse:
348+
errorType = code
349+
msg := gjson.Get(response, "body.message").String()
350+
if msg == "" {
351+
msg = "invalid refresh response"
352+
}
353+
message = msg
342354
}
343-
errorType = common.RefTokenInvalidGrant
344355
}
345356
return errorType, message
346357
}

‎services/oauth/v2/oauth_test.go

+153
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
package v2_test
22

33
import (
4+
"bytes"
45
"fmt"
6+
"io"
57
"math/rand"
68
"net"
79
"net/http"
@@ -576,6 +578,157 @@ var _ = Describe("Oauth", func() {
576578
Expect(response).To(Equal(expectedResponse))
577579
Expect(err).To(MatchError(fmt.Errorf("error occurred while fetching/refreshing account info from CP: mock mock 127.0.0.1:1234->127.0.0.1:12340: read: connection timed out")))
578580
})
581+
582+
It("refreshToken function call when stored cache is same as provided secret and cpApiCall returns a failed response because of faulty implementation in some downstream service", func() {
583+
refreshTokenParams := &v2.RefreshTokenParams{
584+
AccountID: "123",
585+
WorkspaceID: "456",
586+
DestDefName: "testDest",
587+
Destination: Destination,
588+
Secret: []byte(`{"access_token":"storedAccessToken","refresh_token":"dummyRefreshToken","developer_token":"dummyDeveloperToken"}`),
589+
}
590+
591+
ctrl := gomock.NewController(GinkgoT())
592+
mockHttpClient := mockhttpclient.NewMockHttpClient(ctrl)
593+
cpResponseString := `{
594+
"status": 500,
595+
"code": "INVALID_REFRESH_RESPONSE",
596+
"body": {
597+
"code": "INVALID_REFRESH_RESPONSE",
598+
"message": "Missing required token fields in refresh response"
599+
},
600+
"access_token":"storedAccessToken",
601+
"refresh_token":"dummyRefreshToken",
602+
"developer_token":"dummyDeveloperToken"
603+
}`
604+
mockHttpClient.EXPECT().Do(gomock.Any()).Return(&http.Response{
605+
StatusCode: http.StatusOK,
606+
Body: io.NopCloser(bytes.NewBufferString(cpResponseString)),
607+
}, nil)
608+
cpConnector := controlplane.NewConnector(
609+
config.Default,
610+
controlplane.WithClient(mockHttpClient),
611+
controlplane.WithStats(stats.Default),
612+
)
613+
614+
mockTokenProvider := mock_oauthV2.NewMockTokenProvider(ctrl)
615+
mockTokenProvider.EXPECT().Identity().Return(&testutils.BasicAuthMock{})
616+
oauthHandler := v2.NewOAuthHandler(mockTokenProvider,
617+
v2.WithCache(v2.NewCache()),
618+
v2.WithLocker(kitsync.NewPartitionRWLocker()),
619+
v2.WithStats(stats.Default),
620+
v2.WithLogger(logger.NewLogger().Child("MockOAuthHandler")),
621+
v2.WithCpConnector(cpConnector),
622+
)
623+
statusCode, response, err := oauthHandler.RefreshToken(refreshTokenParams)
624+
Expect(statusCode).To(Equal(http.StatusInternalServerError))
625+
expectedResponse := &v2.AuthResponse{
626+
Err: "INVALID_REFRESH_RESPONSE",
627+
ErrorMessage: "Missing required token fields in refresh response",
628+
}
629+
Expect(response).To(Equal(expectedResponse))
630+
Expect(err).To(MatchError(fmt.Errorf("error occurred while fetching/refreshing account info from CP: Missing required token fields in refresh response")))
631+
})
632+
633+
It("refreshToken function call when stored cache is same as provided secret and cpApiCall returns a failed response because of invalid refresh response without message", func() {
634+
refreshTokenParams := &v2.RefreshTokenParams{
635+
AccountID: "123",
636+
WorkspaceID: "456",
637+
DestDefName: "testDest",
638+
Destination: Destination,
639+
Secret: []byte(`{"access_token":"storedAccessToken","refresh_token":"dummyRefreshToken","developer_token":"dummyDeveloperToken"}`),
640+
}
641+
642+
ctrl := gomock.NewController(GinkgoT())
643+
mockHttpClient := mockhttpclient.NewMockHttpClient(ctrl)
644+
cpResponseString := `{
645+
"status": 500,
646+
"code": "INVALID_REFRESH_RESPONSE",
647+
"body": {
648+
"code": "INVALID_REFRESH_RESPONSE"
649+
},
650+
"access_token":"storedAccessToken",
651+
"refresh_token":"dummyRefreshToken",
652+
"developer_token":"dummyDeveloperToken"
653+
}`
654+
mockHttpClient.EXPECT().Do(gomock.Any()).Return(&http.Response{
655+
StatusCode: http.StatusOK,
656+
Body: io.NopCloser(bytes.NewBufferString(cpResponseString)),
657+
}, nil)
658+
cpConnector := controlplane.NewConnector(
659+
config.Default,
660+
controlplane.WithClient(mockHttpClient),
661+
controlplane.WithStats(stats.Default),
662+
)
663+
664+
mockTokenProvider := mock_oauthV2.NewMockTokenProvider(ctrl)
665+
mockTokenProvider.EXPECT().Identity().Return(&testutils.BasicAuthMock{})
666+
oauthHandler := v2.NewOAuthHandler(mockTokenProvider,
667+
v2.WithCache(v2.NewCache()),
668+
v2.WithLocker(kitsync.NewPartitionRWLocker()),
669+
v2.WithStats(stats.Default),
670+
v2.WithLogger(logger.NewLogger().Child("MockOAuthHandler")),
671+
v2.WithCpConnector(cpConnector),
672+
)
673+
statusCode, response, err := oauthHandler.RefreshToken(refreshTokenParams)
674+
Expect(statusCode).To(Equal(http.StatusInternalServerError))
675+
expectedResponse := &v2.AuthResponse{
676+
Err: "INVALID_REFRESH_RESPONSE",
677+
ErrorMessage: "invalid refresh response",
678+
}
679+
Expect(response).To(Equal(expectedResponse))
680+
Expect(err).To(MatchError(fmt.Errorf("error occurred while fetching/refreshing account info from CP: invalid refresh response")))
681+
})
682+
683+
It("refreshToken function call when stored cache is same as provided secret and cpApiCall returns a failed response because of invalid refresh response without message", func() {
684+
refreshTokenParams := &v2.RefreshTokenParams{
685+
AccountID: "123",
686+
WorkspaceID: "456",
687+
DestDefName: "testDest",
688+
Destination: Destination,
689+
Secret: []byte(`{"access_token":"storedAccessToken","refresh_token":"dummyRefreshToken","developer_token":"dummyDeveloperToken"}`),
690+
}
691+
692+
ctrl := gomock.NewController(GinkgoT())
693+
mockHttpClient := mockhttpclient.NewMockHttpClient(ctrl)
694+
cpResponseString := fmt.Sprintf(`{
695+
"status": 500,
696+
"code": "%[1]s",
697+
"body": {
698+
"code": "%[1]s"
699+
},
700+
"access_token":"storedAccessToken",
701+
"refresh_token":"dummyRefreshToken",
702+
"developer_token":"dummyDeveloperToken"
703+
}`, common.RefTokenInvalidGrant)
704+
mockHttpClient.EXPECT().Do(gomock.Any()).Return(&http.Response{
705+
StatusCode: http.StatusOK,
706+
Body: io.NopCloser(bytes.NewBufferString(cpResponseString)),
707+
}, nil)
708+
cpConnector := controlplane.NewConnector(
709+
config.Default,
710+
controlplane.WithClient(mockHttpClient),
711+
controlplane.WithStats(stats.Default),
712+
)
713+
714+
mockTokenProvider := mock_oauthV2.NewMockTokenProvider(ctrl)
715+
mockTokenProvider.EXPECT().Identity().Return(&testutils.BasicAuthMock{})
716+
oauthHandler := v2.NewOAuthHandler(mockTokenProvider,
717+
v2.WithCache(v2.NewCache()),
718+
v2.WithLocker(kitsync.NewPartitionRWLocker()),
719+
v2.WithStats(stats.Default),
720+
v2.WithLogger(logger.NewLogger().Child("MockOAuthHandler")),
721+
v2.WithCpConnector(cpConnector),
722+
)
723+
statusCode, response, err := oauthHandler.RefreshToken(refreshTokenParams)
724+
Expect(statusCode).To(Equal(http.StatusBadRequest))
725+
expectedResponse := &v2.AuthResponse{
726+
Err: common.RefTokenInvalidGrant,
727+
ErrorMessage: v2.ErrPermissionOrTokenRevoked.Error(),
728+
}
729+
Expect(response).To(Equal(expectedResponse))
730+
Expect(err).To(MatchError(fmt.Errorf("invalid grant")))
731+
})
579732
})
580733

581734
Describe("Test AuthStatusToggle function", func() {

0 commit comments

Comments
 (0)