From e76a2dba91010789b5268bfc77888655a190b887 Mon Sep 17 00:00:00 2001 From: Anton Berezhnyi Date: Tue, 7 Nov 2023 12:34:47 +0200 Subject: [PATCH] Add error explanation and metrics. Increase state lifetiem to 6 hours --- ApiController.go | 16 ++++++++-- ApiController_test.go | 70 ++++++++++++++++++++++++++++++++++++------- app.go | 4 --- metrics.go | 12 ++++++++ 4 files changed, 84 insertions(+), 18 deletions(-) create mode 100644 metrics.go diff --git a/ApiController.go b/ApiController.go index 5a72ce0..1840aa1 100644 --- a/ApiController.go +++ b/ApiController.go @@ -21,6 +21,8 @@ import ( const adminUserid = 1 +var stateLifetime = time.Hour * 6 + //go:embed templates/*.html var templates embed.FS @@ -89,7 +91,7 @@ func (controller *ApiController) getAuthUrl(c *gin.Context) { if err == nil { authOptionsClaims.KneuUserId = 0 authOptionsClaims.Issuer = "pigeonAuthorizer" - authOptionsClaims.ExpiresAt = jwt.NewNumericDate(time.Now().Add(time.Minute * 15)) + authOptionsClaims.ExpiresAt = jwt.NewNumericDate(time.Now().Add(stateLifetime)) state, err = controller.buildState(authOptionsClaims) } @@ -119,9 +121,14 @@ func (controller *ApiController) completeAuth(c *gin.Context) { } authOptionsClaims, err := controller.parseState(state) - if err != nil { + if errors.Is(err, jwt.ErrTokenExpired) { + controller.errorResponse(c, "Посилання для авторизації через сайт КНЕУ отримане через бот недійсне. Отримайте нове посилання надіславши боту команду /start") + authErrorExpiredStateTotal.Inc() + return + } else if err != nil { + controller.errorResponse(c, "Некорректне посилання для авторизації через сайт КНЕУ. Отримайте нове посилання надіславши боту команду /start") fmt.Fprintf(controller.out, "Failed to parse state: %s\n", err.Error()) - controller.errorResponse(c, "Невірний стан") + authErrorWrongStateTotal.Inc() return } @@ -129,6 +136,7 @@ func (controller *ApiController) completeAuth(c *gin.Context) { if err != nil { fmt.Fprintf(controller.out, "Failed to get token: %s\n", err.Error()) controller.errorResponse(c, "Помилка отримання токена") + authErrorFailGetTokenTotal.Inc() return } @@ -144,6 +152,7 @@ func (controller *ApiController) completeAuth(c *gin.Context) { if err != nil { fmt.Fprintf(controller.out, "Failed to get user me: %s\n", err.Error()) controller.errorResponse(c, "Помилка отримання даних користувача") + authErrorFailGetUserTotal.Inc() return } @@ -163,6 +172,7 @@ func (controller *ApiController) completeAuth(c *gin.Context) { if err != nil { fmt.Fprintf(controller.out, "Failed to auth user: %s\n", err.Error()) controller.errorResponse(c, "Помилка завершення авторизації") + authErrorFailFinishAuthTotal.Inc() return } else { controller.successRedirect(c, authOptionsClaims) diff --git a/ApiController_test.go b/ApiController_test.go index 2786603..c75669d 100644 --- a/ApiController_test.go +++ b/ApiController_test.go @@ -209,7 +209,7 @@ func TestCompleteAuth(t *testing.T) { authOptionsClaims := AuthOptionsClaims{ RegisteredClaims: jwt.RegisteredClaims{ Issuer: "pigeonAuthorizer", - ExpiresAt: jwt.NewNumericDate(time.Now().Add(time.Minute * 15)), + ExpiresAt: jwt.NewNumericDate(time.Now().Add(stateLifetime)), }, Client: client, ClientUserId: clientUserId, @@ -298,7 +298,7 @@ func TestCompleteAuth(t *testing.T) { authOptionsClaims := AuthOptionsClaims{ RegisteredClaims: jwt.RegisteredClaims{ Issuer: "pigeonAuthorizer", - ExpiresAt: jwt.NewNumericDate(time.Now().Add(time.Minute * 15)), + ExpiresAt: jwt.NewNumericDate(time.Now().Add(stateLifetime)), }, Client: client, ClientUserId: clientUserId, @@ -345,7 +345,7 @@ func TestCompleteAuth(t *testing.T) { authOptionsClaims := AuthOptionsClaims{ RegisteredClaims: jwt.RegisteredClaims{ Issuer: "pigeonAuthorizer", - ExpiresAt: jwt.NewNumericDate(time.Now().Add(time.Minute * 15)), + ExpiresAt: jwt.NewNumericDate(time.Now().Add(stateLifetime)), }, Client: client, ClientUserId: clientUserId, @@ -424,7 +424,7 @@ func TestCompleteAuth(t *testing.T) { authOptionsClaims := AuthOptionsClaims{ RegisteredClaims: jwt.RegisteredClaims{ Issuer: "pigeonAuthorizer", - ExpiresAt: jwt.NewNumericDate(time.Now().Add(time.Minute * 15)), + ExpiresAt: jwt.NewNumericDate(time.Now().Add(stateLifetime)), }, Client: client, ClientUserId: clientUserId, @@ -444,6 +444,54 @@ func TestCompleteAuth(t *testing.T) { assert.Contains(t, w.Body.String(), "Вам потрібно використати особистий кабінет студента") }) + t.Run("error_expired_state", func(t *testing.T) { + client := "telegram" + clientUserId := "999" + finalRedirectUri := "http://example.com" + code := "qwerty1234" + + oauthClient := kneu.NewMockOauthClientInterface(t) + apiClient := kneu.NewMockApiClientInterface(t) + + tokenResponse := kneu.OauthTokenResponse{} + + out := &bytes.Buffer{} + controller := &ApiController{ + out: out, + config: config, + oauthClient: oauthClient, + apiClientFactory: func(token string) kneu.ApiClientInterface { + assert.Equal(t, tokenResponse.AccessToken, token) + return apiClient + }, + countCache: NewCountCache(1), + } + + router := (controller).setupRouter() + + authOptionsClaims := AuthOptionsClaims{ + RegisteredClaims: jwt.RegisteredClaims{ + Issuer: "pigeonAuthorizer", + ExpiresAt: jwt.NewNumericDate(time.Now().Add(-time.Hour * 6)), + }, + Client: client, + ClientUserId: clientUserId, + RedirectUri: finalRedirectUri, + KneuUserId: 0, + } + + state, _ := controller.buildState(authOptionsClaims) + + w := httptest.NewRecorder() + req, _ := http.NewRequest(http.MethodGet, "/complete?code="+code+"&state="+state, nil) + + router.ServeHTTP(w, req) + + assert.Equal(t, http.StatusBadRequest, w.Code) + assert.Contains(t, w.Body.String(), "Не вдалося завершити авторизацію") + assert.Contains(t, w.Body.String(), "Посилання для авторизації через сайт КНЕУ отримане через бот недійсне. Отримайте нове посилання надіславши боту команду /start") + }) + t.Run("error_wrong_state", func(t *testing.T) { code := "qwerty1234" @@ -475,7 +523,7 @@ func TestCompleteAuth(t *testing.T) { assert.Equal(t, http.StatusBadRequest, w.Code) assert.Contains(t, w.Body.String(), "Не вдалося завершити авторизацію") - assert.Contains(t, w.Body.String(), "Невірний стан") + assert.Contains(t, w.Body.String(), "Некорректне посилання для авторизації через сайт КНЕУ") assert.Contains(t, out.String(), "Failed to parse state") }) @@ -511,7 +559,7 @@ func TestCompleteAuth(t *testing.T) { authOptionsClaims := AuthOptionsClaims{ RegisteredClaims: jwt.RegisteredClaims{ Issuer: "pigeonAuthorizer", - ExpiresAt: jwt.NewNumericDate(time.Now().Add(time.Minute * 15)), + ExpiresAt: jwt.NewNumericDate(time.Now().Add(stateLifetime)), }, Client: client, ClientUserId: clientUserId, @@ -572,7 +620,7 @@ func TestCompleteAuth(t *testing.T) { authOptionsClaims := AuthOptionsClaims{ RegisteredClaims: jwt.RegisteredClaims{ Issuer: "pigeonAuthorizer", - ExpiresAt: jwt.NewNumericDate(time.Now().Add(time.Minute * 15)), + ExpiresAt: jwt.NewNumericDate(time.Now().Add(stateLifetime)), }, Client: client, ClientUserId: clientUserId, @@ -667,7 +715,7 @@ func TestCompleteAuth(t *testing.T) { authOptionsClaims := AuthOptionsClaims{ RegisteredClaims: jwt.RegisteredClaims{ Issuer: "pigeonAuthorizer", - ExpiresAt: jwt.NewNumericDate(time.Now().Add(time.Minute * 15)), + ExpiresAt: jwt.NewNumericDate(time.Now().Add(stateLifetime)), }, Client: client, ClientUserId: clientUserId, @@ -716,7 +764,7 @@ func TestCompleteAuth(t *testing.T) { authOptionsClaims := AuthOptionsClaims{ RegisteredClaims: jwt.RegisteredClaims{ Issuer: "pigeonAuthorizer", - ExpiresAt: jwt.NewNumericDate(time.Now().Add(time.Minute * 15)), + ExpiresAt: jwt.NewNumericDate(time.Now().Add(stateLifetime)), }, Client: client, ClientUserId: clientUserId, @@ -789,7 +837,7 @@ func TestCompleteAdminAuth(t *testing.T) { authOptionsClaims := AuthOptionsClaims{ RegisteredClaims: jwt.RegisteredClaims{ Issuer: "pigeonAuthorizer", - ExpiresAt: jwt.NewNumericDate(time.Now().Add(time.Minute * 15)), + ExpiresAt: jwt.NewNumericDate(time.Now().Add(stateLifetime)), }, Client: client, ClientUserId: clientUserId, @@ -824,7 +872,7 @@ func TestCompleteAdminAuth(t *testing.T) { authOptionsClaims := AuthOptionsClaims{ RegisteredClaims: jwt.RegisteredClaims{ Issuer: "pigeonAuthorizer", - ExpiresAt: jwt.NewNumericDate(time.Now().Add(time.Minute * 15)), + ExpiresAt: jwt.NewNumericDate(time.Now().Add(stateLifetime)), }, Client: client, ClientUserId: clientUserId, diff --git a/app.go b/app.go index f1a9452..9618b66 100644 --- a/app.go +++ b/app.go @@ -2,7 +2,6 @@ package main import ( "fmt" - "github.com/VictoriaMetrics/metrics" "github.com/berejant/go-kneu" "github.com/gin-gonic/gin" "github.com/kneu-messenger-pigeon/events" @@ -15,9 +14,6 @@ import ( const ExitCodeMainError = 1 -var getAuthUrlRequestsTotal = metrics.NewCounter("get_auth_url_requests_total") -var completeAuthRequestsTotal = metrics.NewCounter("complete_auth_requests_total") - func runApp(out io.Writer, listenAndServe func(string, http.Handler) error) error { envFilename := "" if _, err := os.Stat(".env"); err == nil { diff --git a/metrics.go b/metrics.go new file mode 100644 index 0000000..c872ae4 --- /dev/null +++ b/metrics.go @@ -0,0 +1,12 @@ +package main + +import "github.com/VictoriaMetrics/metrics" + +var getAuthUrlRequestsTotal = metrics.NewCounter(`get_auth_url_requests_total`) +var completeAuthRequestsTotal = metrics.NewCounter(`complete_auth_requests_total`) + +var authErrorExpiredStateTotal = metrics.NewCounter(`auth_error{error="expired_state"}`) +var authErrorWrongStateTotal = metrics.NewCounter(`auth_error{error="wrong_state"}`) +var authErrorFailGetTokenTotal = metrics.NewCounter(`auth_error{error="fail_get_token"}`) +var authErrorFailGetUserTotal = metrics.NewCounter(`auth_error{error="fail_get_user"}`) +var authErrorFailFinishAuthTotal = metrics.NewCounter(`auth_error{error="fail_finish_auth"}`)