From a90145913cf813fe2c60efdf730c889af79890d2 Mon Sep 17 00:00:00 2001 From: Xabier Larrakoetxea Date: Sun, 10 Nov 2024 13:05:19 +0100 Subject: [PATCH] Remove disable from domain logic and act as unknown token Signed-off-by: Xabier Larrakoetxea --- CHANGELOG.md | 4 ++++ internal/app/auth/auth.go | 1 - internal/app/auth/auth_test.go | 15 --------------- internal/app/auth/token_validate.go | 11 ----------- internal/model/model.go | 1 - internal/storage/memory/mapper.go | 7 ++++--- internal/storage/memory/memory_test.go | 19 +++++++++++++++---- 7 files changed, 23 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 201cc5b..7d7659c 100644 --- 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 index a266ad9..676fb5c 100644 --- 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 index 95580b4..cc7cdfb 100644 --- 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 index eed8da3..d2039a8 100644 --- 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 index 22f3587..f08ff61 100644 --- 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 index 3961036..50f0397 100644 --- 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 index 32f3b5d..b2c33bd 100644 --- a/internal/storage/memory/memory_test.go +++ b/internal/storage/memory/memory_test.go @@ -27,7 +27,6 @@ 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)" @@ -35,6 +34,11 @@ var ( { "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)`), },