Skip to content

Commit 899e0a2

Browse files
xuzhu-591xuzhu-591
committed
fix: review comments
* simplify token generator * rename tokenStorage to tokenStore Signed-off-by: xu.zhu <[email protected]>
1 parent 75628c5 commit 899e0a2

File tree

9 files changed

+78
-99
lines changed

9 files changed

+78
-99
lines changed

‎core/cmd/cmd.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ import (
121121
"github.com/horizoncd/horizon/pkg/regioninformers"
122122
"github.com/horizoncd/horizon/pkg/token/generator"
123123
tokenservice "github.com/horizoncd/horizon/pkg/token/service"
124-
tokenstorage "github.com/horizoncd/horizon/pkg/token/storage"
124+
tokenstore "github.com/horizoncd/horizon/pkg/token/store"
125125
"github.com/horizoncd/horizon/pkg/util/kube"
126126
"github.com/horizoncd/horizon/pkg/workload"
127127

@@ -370,8 +370,8 @@ func Init(ctx context.Context, flags *Flags, coreConfig *config.Config) {
370370
}
371371

372372
oauthAppDAO := oauthdao.NewDAO(mysqlDB)
373-
tokenStorage := tokenstorage.NewStorage(mysqlDB)
374-
oauthManager := oauthmanager.NewManager(oauthAppDAO, tokenStorage,
373+
tokenStore := tokenstore.NewStore(mysqlDB)
374+
oauthManager := oauthmanager.NewManager(oauthAppDAO, tokenStore,
375375
generator.NewAuthorizeGenerator(),
376376
coreConfig.Oauth.AuthorizeCodeExpireIn,
377377
coreConfig.Oauth.AccessTokenExpireIn,

‎core/controller/accesstoken/controller_test.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ import (
2828
"github.com/horizoncd/horizon/pkg/token/generator"
2929
tokenmodels "github.com/horizoncd/horizon/pkg/token/models"
3030
tokenservice "github.com/horizoncd/horizon/pkg/token/service"
31-
tokenstorage "github.com/horizoncd/horizon/pkg/token/storage"
31+
tokenstore "github.com/horizoncd/horizon/pkg/token/store"
3232

3333
"github.com/horizoncd/horizon/core/common"
3434
herror "github.com/horizoncd/horizon/core/errors"
@@ -96,9 +96,9 @@ func TestMain(m *testing.M) {
9696
accessTokenExpireIn := time.Hour * 24
9797
refreshTokenExpireIn := time.Hour * 24 * 30
9898

99-
tokenStorage := tokenstorage.NewStorage(db)
99+
tokenStore := tokenstore.NewStore(db)
100100
oauthAppDAO := oauthdao.NewDAO(db)
101-
oauthMgr := oauthmanager.NewManager(oauthAppDAO, tokenStorage, generator.NewAuthorizeGenerator(),
101+
oauthMgr := oauthmanager.NewManager(oauthAppDAO, tokenStore, generator.NewAuthorizeGenerator(),
102102
authorizeCodeExpireIn, accessTokenExpireIn, refreshTokenExpireIn)
103103

104104
parameter := &param.Param{

‎integrationtest/authpagerender_test.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ import (
4747
"github.com/horizoncd/horizon/pkg/rbac/types"
4848
"github.com/horizoncd/horizon/pkg/token/generator"
4949
tokenmodels "github.com/horizoncd/horizon/pkg/token/models"
50-
tokenstorage "github.com/horizoncd/horizon/pkg/token/storage"
50+
tokenstore "github.com/horizoncd/horizon/pkg/token/store"
5151
callbacks "github.com/horizoncd/horizon/pkg/util/ormcallbacks"
5252
"github.com/stretchr/testify/assert"
5353
"golang.org/x/net/context"
@@ -119,9 +119,9 @@ func TestServer(t *testing.T) {
119119
db = db.WithContext(context.WithValue(context.Background(), common.UserContextKey(), aUser))
120120
callbacks.RegisterCustomCallbacks(db)
121121

122-
tokenStorage := tokenstorage.NewStorage(db)
122+
tokenStore := tokenstore.NewStore(db)
123123
oauthAppDAO := oauthdao.NewDAO(db)
124-
oauthManager := oauthmanager.NewManager(oauthAppDAO, tokenStorage, generator.NewAuthorizeGenerator(),
124+
oauthManager := oauthmanager.NewManager(oauthAppDAO, tokenStore, generator.NewAuthorizeGenerator(),
125125
authorizeCodeExpireIn, accessTokenExpireIn, refreshTokenExpireIn)
126126
clientID := "ho_t65dvkmfqb8v8xzxfbc5"
127127
clientIDGen := func(appType models.AppType) string {

‎pkg/oauth/manager/manager.go

+16-16
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import (
2626
"github.com/horizoncd/horizon/pkg/oauth/models"
2727
"github.com/horizoncd/horizon/pkg/token/generator"
2828
tokenmodels "github.com/horizoncd/horizon/pkg/token/models"
29-
tokenstorage "github.com/horizoncd/horizon/pkg/token/storage"
29+
tokenstore "github.com/horizoncd/horizon/pkg/token/store"
3030
"github.com/horizoncd/horizon/pkg/util/log"
3131
"golang.org/x/net/context"
3232
"k8s.io/apimachinery/pkg/util/rand"
@@ -95,14 +95,14 @@ type Manager interface {
9595

9696
var _ Manager = &OauthManager{}
9797

98-
func NewManager(oauthAppDAO oauthdao.DAO, tokenStorage tokenstorage.Storage,
98+
func NewManager(oauthAppDAO oauthdao.DAO, tokenStore tokenstore.Store,
9999
gen generator.CodeGenerator,
100100
authorizeCodeExpireTime,
101101
accessTokenExpireTime,
102102
refreshTokenExpireTime time.Duration) *OauthManager {
103103
return &OauthManager{
104104
oauthAppDAO: oauthAppDAO,
105-
tokenStorage: tokenStorage,
105+
tokenStore: tokenStore,
106106
authorizationCodeGenerator: gen,
107107
authorizeCodeExpireTime: authorizeCodeExpireTime,
108108
accessTokenExpireTime: accessTokenExpireTime,
@@ -113,7 +113,7 @@ func NewManager(oauthAppDAO oauthdao.DAO, tokenStorage tokenstorage.Storage,
113113

114114
type OauthManager struct {
115115
oauthAppDAO oauthdao.DAO
116-
tokenStorage tokenstorage.Storage
116+
tokenStore tokenstore.Store
117117
authorizationCodeGenerator generator.CodeGenerator
118118
authorizeCodeExpireTime time.Duration
119119
accessTokenExpireTime time.Duration
@@ -170,7 +170,7 @@ func (m *OauthManager) GetOAuthApp(ctx context.Context, clientID string) (*model
170170

171171
func (m *OauthManager) DeleteOAuthApp(ctx context.Context, clientID string) error {
172172
// revoke all the token
173-
if err := m.tokenStorage.DeleteByClientID(ctx, clientID); err != nil {
173+
if err := m.tokenStore.DeleteByClientID(ctx, clientID); err != nil {
174174
return err
175175
}
176176

@@ -316,7 +316,7 @@ func (m *OauthManager) GenAuthorizeCode(ctx context.Context,
316316
}
317317

318318
authorizationToken := m.NewAuthorizationToken(req)
319-
_, err = m.tokenStorage.Create(ctx, authorizationToken)
319+
_, err = m.tokenStore.Create(ctx, authorizationToken)
320320
return authorizationToken, err
321321
}
322322

@@ -339,7 +339,7 @@ func (m *OauthManager) GenOauthTokens(ctx context.Context, req *OauthTokensReque
339339
}
340340

341341
// get authorize token, and check by it
342-
authorizationCodeToken, err := m.tokenStorage.GetByCode(ctx, req.Code)
342+
authorizationCodeToken, err := m.tokenStore.GetByCode(ctx, req.Code)
343343
if err != nil {
344344
if _, ok := perror.Cause(err).(*herrors.HorizonErrNotFound); ok {
345345
return nil, perror.Wrap(err, "authorization code not exist")
@@ -349,7 +349,7 @@ func (m *OauthManager) GenOauthTokens(ctx context.Context, req *OauthTokensReque
349349

350350
if err := m.checkByAuthorizationCode(req, authorizationCodeToken); err != nil {
351351
if perror.Cause(err) == herrors.ErrOAuthCodeExpired {
352-
if delErr := m.tokenStorage.DeleteByCode(ctx, req.Code); delErr != nil {
352+
if delErr := m.tokenStore.DeleteByCode(ctx, req.Code); delErr != nil {
353353
log.Warningf(ctx, "delete expired code error, err = %v", delErr)
354354
}
355355
}
@@ -358,21 +358,21 @@ func (m *OauthManager) GenOauthTokens(ctx context.Context, req *OauthTokensReque
358358

359359
// generate access token and store
360360
accessToken := m.NewAccessToken(authorizationCodeToken, req)
361-
accessTokenInDB, err := m.tokenStorage.Create(ctx, accessToken)
361+
accessTokenInDB, err := m.tokenStore.Create(ctx, accessToken)
362362
if err != nil {
363363
return nil, err
364364
}
365365

366366
// generate refresh token, store and associate with the access token
367367
refreshToken := m.NewRefreshToken(accessToken, req)
368368
refreshToken.RefID = accessTokenInDB.ID
369-
refreshTokenInDB, err := m.tokenStorage.Create(ctx, refreshToken)
369+
refreshTokenInDB, err := m.tokenStore.Create(ctx, refreshToken)
370370
if err != nil {
371371
return nil, err
372372
}
373373

374374
// delete authorize code
375-
err = m.tokenStorage.DeleteByCode(ctx, req.Code)
375+
err = m.tokenStore.DeleteByCode(ctx, req.Code)
376376
if err != nil {
377377
log.Warningf(ctx, "Delete Authorization token error, code = %s, error = %v", req.Code, err)
378378
}
@@ -409,7 +409,7 @@ func (m *OauthManager) RefreshOauthTokens(ctx context.Context,
409409
Request: req.Request,
410410
})
411411
refreshToken.RefID = accessToken.ID
412-
err = m.tokenStorage.UpdateByID(ctx, refreshToken.ID, refreshToken)
412+
err = m.tokenStore.UpdateByID(ctx, refreshToken.ID, refreshToken)
413413
if err != nil {
414414
return nil, err
415415
}
@@ -435,7 +435,7 @@ func (m *OauthManager) checkClientSecret(ctx context.Context, req *OauthTokensRe
435435

436436
func (m *OauthManager) checkRefreshToken(ctx context.Context,
437437
refreshToken, redirectURL string) (*tokenmodels.Token, error) {
438-
token, err := m.tokenStorage.GetByCode(ctx, refreshToken)
438+
token, err := m.tokenStore.GetByCode(ctx, refreshToken)
439439
if err != nil {
440440
if _, ok := perror.Cause(err).(*herrors.HorizonErrNotFound); ok {
441441
return nil, perror.Wrap(err, "refresh token not exist")
@@ -454,7 +454,7 @@ func (m *OauthManager) checkRefreshToken(ctx context.Context,
454454

455455
func (m *OauthManager) refreshAccessToken(ctx context.Context, refreshToken *tokenmodels.Token,
456456
req *OauthTokensRequest) (*tokenmodels.Token, error) {
457-
accessToken, err := m.tokenStorage.GetByID(ctx, refreshToken.RefID)
457+
accessToken, err := m.tokenStore.GetByID(ctx, refreshToken.RefID)
458458
accessTokenNotFound := false
459459
if err != nil {
460460
if _, ok := perror.Cause(err).(*herrors.HorizonErrNotFound); ok {
@@ -471,7 +471,7 @@ func (m *OauthManager) refreshAccessToken(ctx context.Context, refreshToken *tok
471471
Scope: refreshToken.Scope,
472472
UserID: refreshToken.UserID,
473473
}, req)
474-
accessToken, err = m.tokenStorage.Create(ctx, token)
474+
accessToken, err = m.tokenStore.Create(ctx, token)
475475
if err != nil {
476476
return nil, err
477477
}
@@ -482,7 +482,7 @@ func (m *OauthManager) refreshAccessToken(ctx context.Context, refreshToken *tok
482482
Request: req.Request,
483483
})
484484
accessToken.CreatedAt = time.Now()
485-
err = m.tokenStorage.UpdateByID(ctx, accessToken.ID, accessToken)
485+
err = m.tokenStore.UpdateByID(ctx, accessToken.ID, accessToken)
486486
if err != nil {
487487
return nil, err
488488
}

‎pkg/oauth/manager/manager_test.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -34,13 +34,13 @@ import (
3434
"github.com/horizoncd/horizon/pkg/token/generator"
3535
tokenmanager "github.com/horizoncd/horizon/pkg/token/manager"
3636
tokenmodels "github.com/horizoncd/horizon/pkg/token/models"
37-
tokenstorage "github.com/horizoncd/horizon/pkg/token/storage"
37+
tokenstore "github.com/horizoncd/horizon/pkg/token/store"
3838
callbacks "github.com/horizoncd/horizon/pkg/util/ormcallbacks"
3939
)
4040

4141
var (
4242
db *gorm.DB
43-
tokenStorage tokenstorage.Storage
43+
tokenStore tokenstore.Store
4444
oauthAppDAO oauthdao.DAO
4545
oauthManager Manager
4646
tokenManager tokenmanager.Manager
@@ -500,7 +500,7 @@ func TestUpdateToken(t *testing.T) {
500500
tokenInDB.Code = newCode
501501
tokenInDB.CreatedAt = createdAt
502502
tokenInDB.RefID = refID
503-
err = tokenStorage.UpdateByID(ctx, tokenInDB.ID, tokenInDB)
503+
err = tokenStore.UpdateByID(ctx, tokenInDB.ID, tokenInDB)
504504
assert.Nil(t, err)
505505
tokenUpdated, err := tokenManager.LoadTokenByID(ctx, tokenInDB.ID)
506506
assert.Nil(t, err)
@@ -521,9 +521,9 @@ func TestMain(m *testing.M) {
521521
db = db.WithContext(context.WithValue(context.Background(), common.UserContextKey(), aUser))
522522
callbacks.RegisterCustomCallbacks(db)
523523

524-
tokenStorage = tokenstorage.NewStorage(db)
524+
tokenStore = tokenstore.NewStore(db)
525525
oauthAppDAO = oauthdao.NewDAO(db)
526-
oauthManager = NewManager(oauthAppDAO, tokenStorage, generator.NewOauthAccessGenerator(),
526+
oauthManager = NewManager(oauthAppDAO, tokenStore, generator.NewOauthAccessGenerator(),
527527
authorizeCodeExpireIn, accessTokenExpireIn, refreshTokenExpireIn)
528528
tokenManager = tokenmanager.New(db)
529529
os.Exit(m.Run())

‎pkg/token/generator/generator.go

+19-40
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"time"
2323

2424
"github.com/google/uuid"
25+
"k8s.io/apimachinery/pkg/util/rand"
2526
)
2627

2728
// ref: https://github.blog/2021-04-05-behind-githubs-new-authentication-token-formats/
@@ -33,40 +34,32 @@ const (
3334
)
3435

3536
func NewAuthorizeGenerator() CodeGenerator {
36-
return &AuthorizeGenerator{}
37+
return &authorizationCodeGenerator{}
3738
}
3839

3940
func NewHorizonAppUserToServerAccessGenerator() CodeGenerator {
40-
return &BasicAccessTokenGenerator{prefix: HorizonAppUserToServerAccessTokenPrefix}
41+
return &basicTokenGenerator{prefix: HorizonAppUserToServerAccessTokenPrefix}
4142
}
4243

4344
func NewOauthAccessGenerator() CodeGenerator {
44-
return &BasicAccessTokenGenerator{prefix: OauthAPPAccessTokenPrefix}
45+
return &basicTokenGenerator{prefix: OauthAPPAccessTokenPrefix}
4546
}
4647

4748
func NewGeneralAccessTokenGenerator() CodeGenerator {
48-
return &GeneralAccessTokenGenerator{prefix: AccessTokenPrefix}
49+
return &basicTokenGenerator{prefix: AccessTokenPrefix}
4950
}
5051

5152
func NewRefreshTokenGenerator() CodeGenerator {
52-
return &BasicRefreshTokenGenerator{prefix: RefreshTokenPrefix}
53+
return &basicTokenGenerator{prefix: RefreshTokenPrefix}
5354
}
5455

55-
type AuthorizeGenerator struct{}
56+
type authorizationCodeGenerator struct{}
5657

57-
type BasicAccessTokenGenerator struct {
58+
type basicTokenGenerator struct {
5859
prefix string
5960
}
6061

61-
type GeneralAccessTokenGenerator struct {
62-
prefix string
63-
}
64-
65-
type BasicRefreshTokenGenerator struct {
66-
prefix string
67-
}
68-
69-
func (g *AuthorizeGenerator) Generate(info *CodeGenerateInfo) string {
62+
func (g *authorizationCodeGenerator) Generate(info *CodeGenerateInfo) string {
7063
buf := bytes.NewBufferString(info.Token.ClientID)
7164
buf.WriteString(strconv.Itoa(int(info.Token.UserID)))
7265
token := uuid.NewMD5(uuid.Must(uuid.NewRandom()), buf.Bytes())
@@ -75,30 +68,16 @@ func (g *AuthorizeGenerator) Generate(info *CodeGenerateInfo) string {
7568
return code
7669
}
7770

78-
func (g *BasicAccessTokenGenerator) Generate(info *CodeGenerateInfo) string {
79-
access := encodeInfoWithClient(info)
80-
code := g.prefix + strings.ToUpper(strings.TrimRight(access, "="))
81-
return code
82-
}
83-
84-
func (g *GeneralAccessTokenGenerator) Generate(info *CodeGenerateInfo) string {
85-
buf := bytes.NewBufferString(time.Now().String())
86-
buf.WriteString(strconv.Itoa(int(info.Token.UserID)))
87-
code := base64.URLEncoding.EncodeToString([]byte(uuid.NewMD5(uuid.Must(uuid.NewRandom()), buf.Bytes()).String()))
88-
code = g.prefix + strings.ToUpper(strings.TrimRight(code, "="))
89-
return code
90-
}
91-
92-
func (g *BasicRefreshTokenGenerator) Generate(info *CodeGenerateInfo) string {
93-
access := encodeInfoWithClient(info)
94-
code := g.prefix + strings.ToUpper(strings.TrimRight(access, "="))
95-
return code
96-
}
97-
98-
func encodeInfoWithClient(info *CodeGenerateInfo) string {
99-
buf := bytes.NewBufferString(info.Token.ClientID)
71+
func (g *basicTokenGenerator) Generate(info *CodeGenerateInfo) string {
72+
clientID := func(info *CodeGenerateInfo) string {
73+
if info.Token.ClientID != "" {
74+
return info.Token.ClientID
75+
}
76+
return rand.String(20)
77+
}(info)
78+
buf := bytes.NewBufferString(clientID)
10079
buf.WriteString(strconv.Itoa(int(info.Token.UserID)))
101-
buf.WriteString(strconv.FormatInt(info.Token.CreatedAt.UnixNano(), 10))
80+
buf.WriteString(strconv.FormatInt(time.Now().UnixNano(), 10))
10281
access := base64.URLEncoding.EncodeToString([]byte(uuid.NewMD5(uuid.Must(uuid.NewRandom()), buf.Bytes()).String()))
103-
return access
82+
return g.prefix + strings.ToUpper(strings.TrimRight(access, "="))
10483
}

‎pkg/token/manager/manager.go

+8-8
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import (
1818
"context"
1919

2020
"github.com/horizoncd/horizon/pkg/token/models"
21-
"github.com/horizoncd/horizon/pkg/token/storage"
21+
"github.com/horizoncd/horizon/pkg/token/store"
2222
"gorm.io/gorm"
2323
)
2424

@@ -31,29 +31,29 @@ type Manager interface {
3131
}
3232

3333
func New(db *gorm.DB) Manager {
34-
return &manager{storage: storage.NewStorage(db)}
34+
return &manager{store: store.NewStore(db)}
3535
}
3636

3737
type manager struct {
38-
storage storage.Storage
38+
store store.Store
3939
}
4040

4141
func (m *manager) CreateToken(ctx context.Context, token *models.Token) (*models.Token, error) {
42-
return m.storage.Create(ctx, token)
42+
return m.store.Create(ctx, token)
4343
}
4444

4545
func (m *manager) LoadTokenByID(ctx context.Context, id uint) (*models.Token, error) {
46-
return m.storage.GetByID(ctx, id)
46+
return m.store.GetByID(ctx, id)
4747
}
4848

4949
func (m *manager) LoadTokenByCode(ctx context.Context, code string) (*models.Token, error) {
50-
return m.storage.GetByCode(ctx, code)
50+
return m.store.GetByCode(ctx, code)
5151
}
5252

5353
func (m *manager) RevokeTokenByID(ctx context.Context, id uint) error {
54-
return m.storage.DeleteByID(ctx, id)
54+
return m.store.DeleteByID(ctx, id)
5555
}
5656

5757
func (m *manager) RevokeTokenByClientID(ctx context.Context, clientID string) error {
58-
return m.storage.DeleteByClientID(ctx, clientID)
58+
return m.store.DeleteByClientID(ctx, clientID)
5959
}

0 commit comments

Comments
 (0)