Skip to content

Commit

Permalink
auth, etcdserver: follow the correct usage of context
Browse files Browse the repository at this point in the history
The keys of context shouldn't be string. They should be a struct of
their own type.

Fix etcd-io#8826
  • Loading branch information
mitake committed Nov 21, 2017
1 parent fb9e78f commit 382c5a5
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 13 deletions.
6 changes: 3 additions & 3 deletions auth/simple_token.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions auth/simple_token_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
8 changes: 6 additions & 2 deletions auth/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,10 @@ type AuthInfo struct {
Revision uint64
}

// they are used for the keys of context in the parameters of Authenticate()
type AuthenticateParamIndex struct{}
type AuthenticateParamSimpleTokenPrefix struct{}

type AuthStore interface {
// AuthEnable turns on the authentication feature
AuthEnable() error
Expand Down Expand Up @@ -1062,13 +1066,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
}
Expand Down
10 changes: 5 additions & 5 deletions auth/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}
Expand Down
3 changes: 2 additions & 1 deletion etcdserver/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 382c5a5

Please sign in to comment.