Remove disable from domain logic and act as unknown token

Signed-off-by: Xabier Larrakoetxea <[email protected]>
slok · Nov 10, 2024 · a901459 · a901459
1 parent 7fe83f9
commit a901459
Show file tree

Hide file tree

Showing 7 changed files with 23 additions and 35 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,10 @@
 
 ## [Unreleased]
 
+### Changed
+
+- Disabled tokens now will act as missing/unknown tokens, they will not return "disabledToken" error nor metric.
+
 ## [v0.4.0] - 2022-12-10
 
 ### Added

diff --git a/internal/app/auth/auth.go b/internal/app/auth/auth.go
@@ -33,7 +33,6 @@ func NewService(logger log.Logger, metricsRec metrics.Recorder, tokenGetter Toke
 
 		authenticater: newAuthenticaterChain(
 			newTokenExistAuthenticator(),
-			newDisabledAuthenticator(),
 			newNotExpiredAuthenticator(),
 			newValidMethodAuthenticator(),
 			newValidURLAuthenticator(),

diff --git a/internal/app/auth/auth_test.go b/internal/app/auth/auth_test.go
@@ -53,21 +53,6 @@ func TestServiceAuth(t *testing.T) {
 			expResp: &auth.AuthenticateResponse{Authenticated: false, Reason: auth.ReasonInvalidToken},
 		},
 
-		"A token review that is disabled should be invalid.": {
-			mock: func(mtg *authmock.TokenGetter) {
-				mtg.On("GetStaticTokenValidation", mock.Anything, "token0").Once().Return(&model.StaticTokenValidation{
-					Value: "token0",
-					Common: model.TokenCommon{
-						Disable: true,
-					},
-				}, nil)
-			},
-			req: auth.AuthenticateRequest{Review: model.TokenReview{
-				Token: "token0",
-			}},
-			expResp: &auth.AuthenticateResponse{Authenticated: false, Reason: auth.ReasonDisabledToken},
-		},
-
 		"A token review that has expired should be invalid.": {
 			mock: func(mtg *authmock.TokenGetter) {
 				mtg.On("GetStaticTokenValidation", mock.Anything, "token0").Once().Return(&model.StaticTokenValidation{

diff --git a/internal/app/auth/token_validate.go b/internal/app/auth/token_validate.go
@@ -12,7 +12,6 @@ const (
 	ReasonExpiredToken  = "expiredToken"
 	ReasonInvalidURL    = "invalidURL"
 	ReasonInvalidMethod = "invalidMethod"
-	ReasonDisabledToken = "disabledToken"
 )
 
 type reviewResult struct {
@@ -102,13 +101,3 @@ func newValidURLAuthenticator() authenticater {
 		return &reviewResult{Valid: false, Reason: ReasonInvalidURL}, nil
 	})
 }
-
-func newDisabledAuthenticator() authenticater {
-	return authenticaterFunc(func(ctx context.Context, r model.TokenReview, t model.StaticTokenValidation) (*reviewResult, error) {
-		if !t.Common.Disable {
-			return &reviewResult{Valid: true}, nil
-		}
-
-		return &reviewResult{Valid: false, Reason: ReasonDisabledToken}, nil
-	})
-}
diff --git a/internal/model/model.go b/internal/model/model.go
@@ -15,7 +15,6 @@ type StaticTokenValidation struct {
 }
 
 type TokenCommon struct {
-	Disable       bool
 	AllowedURL    *regexp.Regexp
 	AllowedMethod *regexp.Regexp
 }

diff --git a/internal/storage/memory/mapper.go b/internal/storage/memory/mapper.go
@@ -41,6 +41,10 @@ func mapJSONV1ToModel(data string) (map[string]model.StaticTokenValidation, erro
 			return nil, fmt.Errorf("token value can't be empty")
 		}
 
+		if t.Disable {
+			continue
+		}
+
 		var expiresAt time.Time
 		if t.ExpiresAt != nil {
 			expiresAt = *t.ExpiresAt
@@ -50,9 +54,6 @@ func mapJSONV1ToModel(data string) (map[string]model.StaticTokenValidation, erro
 			Value:     t.Value,
 			ClientID:  t.ClientID,
 			ExpiresAt: expiresAt,
-			Common: model.TokenCommon{
-				Disable: t.Disable,
-			},
 		}
 
 		if t.AllowedMethodRegex != "" {

diff --git a/internal/storage/memory/memory_test.go b/internal/storage/memory/memory_test.go
@@ -27,14 +27,18 @@ var (
 		{
 			"value": "t1",
 			"client_id": "c1",
-			"disable": true,
 			"expires_at": "2022-07-04T14:21:22.52Z",
 			"allowed_url": "https://custom.host.slok.dev/.*",
 			"allowed_method": "(GET|POST)"
 		},
 		{
 			"value": "t2",
 			"allowed_method": "PUT"
+		},
+		{
+			"value": "t3",
+			"disable": true,
+			"allowed_method": "PUT"
 		}
 	]
 }
@@ -47,13 +51,16 @@ tokens:
 
 - value: t1
   client_id: c1
-  disable: true
   expires_at: 2022-07-04T14:21:22.52Z
   allowed_url: https://custom.host.slok.dev/.*
   allowed_method: (GET|POST)
 
 - value: t2
   allowed_method: PUT
+
+- value: t3
+  disable: true
+  allowed_method: PUT
 `
 )
 
@@ -66,6 +73,12 @@ func TestTokenRepositoryGetStaticTokenValidation(t *testing.T) {
 		expErr   bool
 	}{
 		"If the token is missing, it should fail": {
+			config: goodJSONConfig,
+			token:  "t4",
+			expErr: true,
+		},
+
+		"If the token is disabled, it should fail": {
 			config: goodJSONConfig,
 			token:  "t3",
 			expErr: true,
@@ -88,7 +101,6 @@ func TestTokenRepositoryGetStaticTokenValidation(t *testing.T) {
 				ClientID:  "c1",
 				ExpiresAt: time.Date(2022, time.Month(7), 4, 14, 21, 22, 520000000, time.UTC),
 				Common: model.TokenCommon{
-					Disable:       true,
 					AllowedURL:    regexp.MustCompile(`https://custom.host.slok.dev/.*`),
 					AllowedMethod: regexp.MustCompile(`(GET|POST)`),
 				},
@@ -103,7 +115,6 @@ func TestTokenRepositoryGetStaticTokenValidation(t *testing.T) {
 				ClientID:  "c1",
 				ExpiresAt: time.Date(2022, time.Month(7), 4, 14, 21, 22, 520000000, time.UTC),
 				Common: model.TokenCommon{
-					Disable:       true,
 					AllowedURL:    regexp.MustCompile(`https://custom.host.slok.dev/.*`),
 					AllowedMethod: regexp.MustCompile(`(GET|POST)`),
 				},