From f649132a5a41afee02a29c36d826e789bd144684 Mon Sep 17 00:00:00 2001 From: Hitoshi Mitake Date: Tue, 21 Nov 2017 15:00:13 +0900 Subject: [PATCH] auth, etcdserver: follow the correct usage of context The keys of context shouldn't be string. They should be a struct of their own type. Fix https://github.com/coreos/etcd/issues/8826 --- auth/simple_token.go | 6 +++--- auth/simple_token_test.go | 4 ++-- auth/store.go | 10 ++++++++-- auth/store_test.go | 10 +++++----- etcdserver/apply.go | 3 ++- 5 files changed, 20 insertions(+), 13 deletions(-) diff --git a/auth/simple_token.go b/auth/simple_token.go index 225791d8596..ac55ad7f13f 100644 --- a/auth/simple_token.go +++ b/auth/simple_token.go @@ -188,9 +188,9 @@ func (t *tokenSimple) info(ctx context.Context, token string, revision uint64) ( func (t *tokenSimple) assign(ctx context.Context, username string, rev uint64) (string, error) { // rev isn't used in simple token, it is only used in JWT - index := ctx.Value("index").(uint64) - simpleToken := ctx.Value("simpleToken").(string) - token := fmt.Sprintf("%s.%d", simpleToken, index) + index := ctx.Value(AuthenticateParamIndex{}).(uint64) + simpleTokenPrefix := ctx.Value(AuthenticateParamSimpleTokenPrefix{}).(string) + token := fmt.Sprintf("%s.%d", simpleTokenPrefix, index) t.assignSimpleTokenToUser(username, token) return token, nil diff --git a/auth/simple_token_test.go b/auth/simple_token_test.go index 862b88493fe..1890521d4ab 100644 --- a/auth/simple_token_test.go +++ b/auth/simple_token_test.go @@ -29,7 +29,7 @@ func TestSimpleTokenDisabled(t *testing.T) { explicitlyDisabled.disable() for _, tp := range []*tokenSimple{initialState, explicitlyDisabled} { - ctx := context.WithValue(context.WithValue(context.TODO(), "index", uint64(1)), "simpleToken", "dummy") + ctx := context.WithValue(context.WithValue(context.TODO(), AuthenticateParamIndex{}, uint64(1)), AuthenticateParamSimpleTokenPrefix{}, "dummy") token, err := tp.assign(ctx, "user1", 0) if err != nil { t.Fatal(err) @@ -48,7 +48,7 @@ func TestSimpleTokenDisabled(t *testing.T) { func TestSimpleTokenAssign(t *testing.T) { tp := newTokenProviderSimple(dummyIndexWaiter) tp.enable() - ctx := context.WithValue(context.WithValue(context.TODO(), "index", uint64(1)), "simpleToken", "dummy") + ctx := context.WithValue(context.WithValue(context.TODO(), AuthenticateParamIndex{}, uint64(1)), AuthenticateParamSimpleTokenPrefix{}, "dummy") token, err := tp.assign(ctx, "user1", 0) if err != nil { t.Fatal(err) diff --git a/auth/store.go b/auth/store.go index 363456cb26b..ddb6f73838b 100644 --- a/auth/store.go +++ b/auth/store.go @@ -81,6 +81,12 @@ type AuthInfo struct { Revision uint64 } +// AuthenticateParamIndex is used for a key of context in the parameters of Authenticate() +type AuthenticateParamIndex struct{} + +// AuthenticateParamSimpleTokenPrefix is used for a key of context in the parameters of Authenticate() +type AuthenticateParamSimpleTokenPrefix struct{} + type AuthStore interface { // AuthEnable turns on the authentication feature AuthEnable() error @@ -1062,13 +1068,13 @@ func (as *authStore) WithRoot(ctx context.Context) context.Context { var ctxForAssign context.Context if ts := as.tokenProvider.(*tokenSimple); ts != nil { - ctx1 := context.WithValue(ctx, "index", uint64(0)) + ctx1 := context.WithValue(ctx, AuthenticateParamIndex{}, uint64(0)) prefix, err := ts.genTokenPrefix() if err != nil { plog.Errorf("failed to generate prefix of internally used token") return ctx } - ctxForAssign = context.WithValue(ctx1, "simpleToken", prefix) + ctxForAssign = context.WithValue(ctx1, AuthenticateParamSimpleTokenPrefix{}, prefix) } else { ctxForAssign = ctx } diff --git a/auth/store_test.go b/auth/store_test.go index 834274d273a..b5e784d7f3f 100644 --- a/auth/store_test.go +++ b/auth/store_test.go @@ -199,7 +199,7 @@ func TestUserChangePassword(t *testing.T) { as, tearDown := setupAuthStore(t) defer tearDown(t) - ctx1 := context.WithValue(context.WithValue(context.TODO(), "index", uint64(1)), "simpleToken", "dummy") + ctx1 := context.WithValue(context.WithValue(context.TODO(), AuthenticateParamIndex{}, uint64(1)), AuthenticateParamSimpleTokenPrefix{}, "dummy") _, err := as.Authenticate(ctx1, "foo", "bar") if err != nil { t.Fatal(err) @@ -210,7 +210,7 @@ func TestUserChangePassword(t *testing.T) { t.Fatal(err) } - ctx2 := context.WithValue(context.WithValue(context.TODO(), "index", uint64(2)), "simpleToken", "dummy") + ctx2 := context.WithValue(context.WithValue(context.TODO(), AuthenticateParamIndex{}, uint64(2)), AuthenticateParamSimpleTokenPrefix{}, "dummy") _, err = as.Authenticate(ctx2, "foo", "baz") if err != nil { t.Fatal(err) @@ -461,7 +461,7 @@ func TestAuthInfoFromCtx(t *testing.T) { t.Errorf("expected (nil, nil), got (%v, %v)", ai, err) } - ctx = context.WithValue(context.WithValue(context.TODO(), "index", uint64(1)), "simpleToken", "dummy") + ctx = context.WithValue(context.WithValue(context.TODO(), AuthenticateParamIndex{}, uint64(1)), AuthenticateParamSimpleTokenPrefix{}, "dummy") resp, err := as.Authenticate(ctx, "foo", "bar") if err != nil { t.Error(err) @@ -494,7 +494,7 @@ func TestAuthDisable(t *testing.T) { defer tearDown(t) as.AuthDisable() - ctx := context.WithValue(context.WithValue(context.TODO(), "index", uint64(2)), "simpleToken", "dummy") + ctx := context.WithValue(context.WithValue(context.TODO(), AuthenticateParamIndex{}, uint64(2)), AuthenticateParamSimpleTokenPrefix{}, "dummy") _, err := as.Authenticate(ctx, "foo", "bar") if err != ErrAuthNotEnabled { t.Errorf("expected %v, got %v", ErrAuthNotEnabled, err) @@ -642,7 +642,7 @@ func TestHammerSimpleAuthenticate(t *testing.T) { go func(user string) { defer wg.Done() token := fmt.Sprintf("%s(%d)", user, i) - ctx := context.WithValue(context.WithValue(context.TODO(), "index", uint64(1)), "simpleToken", token) + ctx := context.WithValue(context.WithValue(context.TODO(), AuthenticateParamIndex{}, uint64(1)), AuthenticateParamSimpleTokenPrefix{}, token) if _, err := as.Authenticate(ctx, user, "123"); err != nil { t.Fatal(err) } diff --git a/etcdserver/apply.go b/etcdserver/apply.go index dfd09a464c9..ce4bfbfbcc9 100644 --- a/etcdserver/apply.go +++ b/etcdserver/apply.go @@ -20,6 +20,7 @@ import ( "sort" "time" + "github.com/coreos/etcd/auth" pb "github.com/coreos/etcd/etcdserver/etcdserverpb" "github.com/coreos/etcd/lease" "github.com/coreos/etcd/mvcc" @@ -652,7 +653,7 @@ func (a *applierV3backend) AuthDisable() (*pb.AuthDisableResponse, error) { } func (a *applierV3backend) Authenticate(r *pb.InternalAuthenticateRequest) (*pb.AuthenticateResponse, error) { - ctx := context.WithValue(context.WithValue(a.s.ctx, "index", a.s.consistIndex.ConsistentIndex()), "simpleToken", r.SimpleToken) + ctx := context.WithValue(context.WithValue(a.s.ctx, auth.AuthenticateParamIndex{}, a.s.consistIndex.ConsistentIndex()), auth.AuthenticateParamSimpleTokenPrefix{}, r.SimpleToken) resp, err := a.s.AuthStore().Authenticate(ctx, r.Name, r.Password) if resp != nil { resp.Header = newHeader(a.s)