From e5de8470793be90c1f22dbd98efcb063c6afc102 Mon Sep 17 00:00:00 2001 From: Petyo Stoyanov Date: Wed, 12 Nov 2025 09:40:24 +0200 Subject: [PATCH 1/4] Update CHANGELOG.md --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2457e838d..b9a38e45f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## Unreleased +### Changed +- Improve refresh token grace period logging [#814](https://github.com/rokwire/core-building-block/issues/814) + ## [1.60.0] - 2025-11-110 ### Changed - Implement refresh token reuse detection grace period [#811](https://github.com/rokwire/core-building-block/issues/811) From 38985cafdf9db1fe631821d7a4db3e6fd24b4909 Mon Sep 17 00:00:00 2001 From: Petyo Stoyanov Date: Wed, 12 Nov 2025 10:17:20 +0200 Subject: [PATCH 2/4] Better logging --- .secrets.baseline | 4 ++-- core/auth/apis.go | 34 ++++++++++++++++++++++++++++++---- 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/.secrets.baseline b/.secrets.baseline index 5b042a7db..f04838736 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -151,7 +151,7 @@ "filename": "core/auth/apis.go", "hashed_secret": "394e3412459f79523e12e1fa95a4cf141ccff122", "is_verified": false, - "line_number": 2249 + "line_number": 2275 } ], "core/auth/auth.go": [ @@ -347,5 +347,5 @@ } ] }, - "generated_at": "2025-11-05T17:05:03Z" + "generated_at": "2025-11-12T08:17:11Z" } diff --git a/core/auth/apis.go b/core/auth/apis.go index 4d170b31c..1a34378fd 100644 --- a/core/auth/apis.go +++ b/core/auth/apis.go @@ -309,22 +309,48 @@ func (a *Auth) Refresh(refreshToken string, apiKey string, clientVersion *string } if refreshToken != currentToken { //allow refresh if the previous token is being used and we are in the grace period + + l.Infof("old refresh token being used - %s", refreshToken) + + //get the previous token as it is what we allow during grace period previousToken, err := loginSession.PreviousRefreshToken() - gracePeriodRefresh := (err == nil) && (refreshToken == previousToken) && loginSession.IsInRefreshGracePeriod(nil) - if !gracePeriodRefresh { - l.Infof("previous refresh token being used, so delete login session and return null - %s", refreshToken) + if err != nil { + //we cannot get the previous token, so we cannot allow refresh + l.Infof("error getting previous refresh token - %v", err) + + //remove the session + err = a.deleteLoginSession(nil, *loginSession, l) if err != nil { - l.Errorf("error getting previous refresh token - %v", err) + return nil, errors.WrapErrorAction(logutils.ActionDelete, model.TypeLoginSession, logutils.StringArgs("previous refresh token"), err) } + return nil, nil + } + //now check if the provided token is the previous token + if refreshToken != previousToken { + //not the previous token, so we cannot allow refresh + l.Infof("not the previous token, so we cannot allow refresh - %s", refreshToken) //remove the session err = a.deleteLoginSession(nil, *loginSession, l) if err != nil { return nil, errors.WrapErrorAction(logutils.ActionDelete, model.TypeLoginSession, logutils.StringArgs("previous refresh token"), err) } + return nil, nil + } + //now check if we are in the grace period + if !loginSession.IsInRefreshGracePeriod(nil) { + l.Infof("not in grace period, so we cannot allow refresh - %s", refreshToken) + //remove the session + err = a.deleteLoginSession(nil, *loginSession, l) + if err != nil { + return nil, errors.WrapErrorAction(logutils.ActionDelete, model.TypeLoginSession, logutils.StringArgs("previous refresh token"), err) + } return nil, nil } + + //the provided token is the previous token and we are in grace period, so allow refresh + l.Info("the provided token is the previous token and we are in grace period, so allow refresh") } //TODO: Ideally we would not make many database calls before validating the API key. Currently needed to get app ID From 3d81a46598d87813b3ccc4f1f7a5cc460dadaa27 Mon Sep 17 00:00:00 2001 From: Petyo Stoyanov Date: Wed, 12 Nov 2025 10:31:17 +0200 Subject: [PATCH 3/4] Mask when log sensitive data --- core/auth/apis.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/auth/apis.go b/core/auth/apis.go index 1a34378fd..898a01ad4 100644 --- a/core/auth/apis.go +++ b/core/auth/apis.go @@ -310,7 +310,7 @@ func (a *Auth) Refresh(refreshToken string, apiKey string, clientVersion *string if refreshToken != currentToken { //allow refresh if the previous token is being used and we are in the grace period - l.Infof("old refresh token being used - %s", refreshToken) + l.Infof("old refresh token being used - %s", utils.MaskString(refreshToken, 5)) //get the previous token as it is what we allow during grace period previousToken, err := loginSession.PreviousRefreshToken() @@ -328,7 +328,7 @@ func (a *Auth) Refresh(refreshToken string, apiKey string, clientVersion *string //now check if the provided token is the previous token if refreshToken != previousToken { //not the previous token, so we cannot allow refresh - l.Infof("not the previous token, so we cannot allow refresh - %s", refreshToken) + l.Infof("not the previous token, so we cannot allow refresh - %s", utils.MaskString(refreshToken, 5)) //remove the session err = a.deleteLoginSession(nil, *loginSession, l) @@ -339,7 +339,7 @@ func (a *Auth) Refresh(refreshToken string, apiKey string, clientVersion *string } //now check if we are in the grace period if !loginSession.IsInRefreshGracePeriod(nil) { - l.Infof("not in grace period, so we cannot allow refresh - %s", refreshToken) + l.Infof("not in grace period, so we cannot allow refresh - %s", utils.MaskString(refreshToken, 5)) //remove the session err = a.deleteLoginSession(nil, *loginSession, l) From dc229327226a5c5f3baef9c97cef7961c8d22411 Mon Sep 17 00:00:00 2001 From: Petyo Stoyanov Date: Wed, 12 Nov 2025 10:42:57 +0200 Subject: [PATCH 4/4] More structured code --- .secrets.baseline | 4 ++-- core/auth/apis.go | 42 ++++++++++++++++++------------------------ 2 files changed, 20 insertions(+), 26 deletions(-) diff --git a/.secrets.baseline b/.secrets.baseline index f04838736..c2c58082b 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -151,7 +151,7 @@ "filename": "core/auth/apis.go", "hashed_secret": "394e3412459f79523e12e1fa95a4cf141ccff122", "is_verified": false, - "line_number": 2275 + "line_number": 2269 } ], "core/auth/auth.go": [ @@ -347,5 +347,5 @@ } ] }, - "generated_at": "2025-11-12T08:17:11Z" + "generated_at": "2025-11-12T08:42:48Z" } diff --git a/core/auth/apis.go b/core/auth/apis.go index 898a01ad4..3df770b17 100644 --- a/core/auth/apis.go +++ b/core/auth/apis.go @@ -310,43 +310,37 @@ func (a *Auth) Refresh(refreshToken string, apiKey string, clientVersion *string if refreshToken != currentToken { //allow refresh if the previous token is being used and we are in the grace period - l.Infof("old refresh token being used - %s", utils.MaskString(refreshToken, 5)) + masked := utils.MaskString(refreshToken, 5) + l.Infof("old refresh token being used - %s", masked) - //get the previous token as it is what we allow during grace period - previousToken, err := loginSession.PreviousRefreshToken() - if err != nil { - //we cannot get the previous token, so we cannot allow refresh - l.Infof("error getting previous refresh token - %v", err) - - //remove the session + //small helper to avoid repeating cleanup + cleanup := func() (*model.LoginSession, error) { err = a.deleteLoginSession(nil, *loginSession, l) if err != nil { return nil, errors.WrapErrorAction(logutils.ActionDelete, model.TypeLoginSession, logutils.StringArgs("previous refresh token"), err) } return nil, nil } + + //get the previous token as it is what we allow during grace period + previousToken, err := loginSession.PreviousRefreshToken() + if err != nil { + //we cannot get the previous token, so we cannot allow refresh + l.Infof("error getting previous refresh token - %v", err) + return cleanup() + } + //now check if the provided token is the previous token if refreshToken != previousToken { //not the previous token, so we cannot allow refresh - l.Infof("not the previous token, so we cannot allow refresh - %s", utils.MaskString(refreshToken, 5)) - - //remove the session - err = a.deleteLoginSession(nil, *loginSession, l) - if err != nil { - return nil, errors.WrapErrorAction(logutils.ActionDelete, model.TypeLoginSession, logutils.StringArgs("previous refresh token"), err) - } - return nil, nil + l.Infof("not the previous token, so we cannot allow refresh - %s", masked) + return cleanup() } + //now check if we are in the grace period if !loginSession.IsInRefreshGracePeriod(nil) { - l.Infof("not in grace period, so we cannot allow refresh - %s", utils.MaskString(refreshToken, 5)) - - //remove the session - err = a.deleteLoginSession(nil, *loginSession, l) - if err != nil { - return nil, errors.WrapErrorAction(logutils.ActionDelete, model.TypeLoginSession, logutils.StringArgs("previous refresh token"), err) - } - return nil, nil + l.Infof("not in grace period, so we cannot allow refresh - %s", masked) + return cleanup() } //the provided token is the previous token and we are in grace period, so allow refresh