Skip to content

Commit

Permalink
Merge pull request #9654 from gyuho/auth
Browse files Browse the repository at this point in the history 
auth: support structured logging
  • Loading branch information
@gyuho
gyuho committed Apr 27, 2018
2 parents e83cc21 + 5fd9270 commit 4bab1e1
Show file tree
Hide file tree
Showing 10 changed files with 456 additions and 129 deletions.
104 changes: 90 additions & 14 deletions auth/jwt.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,11 @@ import (
"time"

jwt "github.com/dgrijalva/jwt-go"
"go.uber.org/zap"
)

type tokenJWT struct {
lg *zap.Logger
signMethod string
signKey *rsa.PrivateKey
verifyKey *rsa.PublicKey
Expand All @@ -49,7 +51,11 @@ func (t *tokenJWT) info(ctx context.Context, token string, rev uint64) (*AuthInf
switch err.(type) {
case nil:
if !parsed.Valid {
plog.Warningf("invalid jwt token: %s", token)
if t.lg != nil {
t.lg.Warn("invalid JWT token", zap.String("token", token))
} else {
plog.Warningf("invalid jwt token: %s", token)
}
return nil, false
}

Expand All @@ -58,7 +64,15 @@ func (t *tokenJWT) info(ctx context.Context, token string, rev uint64) (*AuthInf
username = claims["username"].(string)
revision = uint64(claims["revision"].(float64))
default:
plog.Warningf("failed to parse jwt token: %s", err)
if t.lg != nil {
t.lg.Warn(
"failed to parse a JWT token",
zap.String("token", token),
zap.Error(err),
)
} else {
plog.Warningf("failed to parse jwt token: %s", err)
}
return nil, false
}

Expand All @@ -77,16 +91,33 @@ func (t *tokenJWT) assign(ctx context.Context, username string, revision uint64)

token, err := tk.SignedString(t.signKey)
if err != nil {
plog.Debugf("failed to sign jwt token: %s", err)
if t.lg != nil {
t.lg.Warn(
"failed to sign a JWT token",
zap.String("user-name", username),
zap.Uint64("revision", revision),
zap.Error(err),
)
} else {
plog.Debugf("failed to sign jwt token: %s", err)
}
return "", err
}

plog.Debugf("jwt token: %s", token)

if t.lg != nil {
t.lg.Info(
"created/assigned a new JWT token",
zap.String("user-name", username),
zap.Uint64("revision", revision),
zap.String("token", token),
)
} else {
plog.Debugf("jwt token: %s", token)
}
return token, err
}

func prepareOpts(opts map[string]string) (jwtSignMethod, jwtPubKeyPath, jwtPrivKeyPath string, ttl time.Duration, err error) {
func prepareOpts(lg *zap.Logger, opts map[string]string) (jwtSignMethod, jwtPubKeyPath, jwtPrivKeyPath string, ttl time.Duration, err error) {
for k, v := range opts {
switch k {
case "sign-method":
Expand All @@ -98,11 +129,23 @@ func prepareOpts(opts map[string]string) (jwtSignMethod, jwtPubKeyPath, jwtPrivK
case "ttl":
ttl, err = time.ParseDuration(v)
if err != nil {
plog.Errorf("failed to parse ttl option (%s)", err)
if lg != nil {
lg.Warn(
"failed to parse JWT TTL option",
zap.String("ttl-value", v),
zap.Error(err),
)
} else {
plog.Errorf("failed to parse ttl option (%s)", err)
}
return "", "", "", 0, ErrInvalidAuthOpts
}
default:
plog.Errorf("unknown token specific option: %s", k)
if lg != nil {
lg.Warn("unknown JWT token option", zap.String("option", k))
} else {
plog.Errorf("unknown token specific option: %s", k)
}
return "", "", "", 0, ErrInvalidAuthOpts
}
}
Expand All @@ -112,8 +155,8 @@ func prepareOpts(opts map[string]string) (jwtSignMethod, jwtPubKeyPath, jwtPrivK
return jwtSignMethod, jwtPubKeyPath, jwtPrivKeyPath, ttl, nil
}

func newTokenProviderJWT(opts map[string]string) (*tokenJWT, error) {
jwtSignMethod, jwtPubKeyPath, jwtPrivKeyPath, ttl, err := prepareOpts(opts)
func newTokenProviderJWT(lg *zap.Logger, opts map[string]string) (*tokenJWT, error) {
jwtSignMethod, jwtPubKeyPath, jwtPrivKeyPath, ttl, err := prepareOpts(lg, opts)
if err != nil {
return nil, ErrInvalidAuthOpts
}
Expand All @@ -123,30 +166,63 @@ func newTokenProviderJWT(opts map[string]string) (*tokenJWT, error) {
}

t := &tokenJWT{
lg: lg,
ttl: ttl,
}

t.signMethod = jwtSignMethod

verifyBytes, err := ioutil.ReadFile(jwtPubKeyPath)
if err != nil {
plog.Errorf("failed to read public key (%s) for jwt: %s", jwtPubKeyPath, err)
if lg != nil {
lg.Warn(
"failed to read JWT public key",
zap.String("public-key-path", jwtPubKeyPath),
zap.Error(err),
)
} else {
plog.Errorf("failed to read public key (%s) for jwt: %s", jwtPubKeyPath, err)
}
return nil, err
}
t.verifyKey, err = jwt.ParseRSAPublicKeyFromPEM(verifyBytes)
if err != nil {
plog.Errorf("failed to parse public key (%s): %s", jwtPubKeyPath, err)
if lg != nil {
lg.Warn(
"failed to parse JWT public key",
zap.String("public-key-path", jwtPubKeyPath),
zap.Error(err),
)
} else {
plog.Errorf("failed to parse public key (%s): %s", jwtPubKeyPath, err)
}
return nil, err
}

signBytes, err := ioutil.ReadFile(jwtPrivKeyPath)
if err != nil {
plog.Errorf("failed to read private key (%s) for jwt: %s", jwtPrivKeyPath, err)
if lg != nil {
lg.Warn(
"failed to read JWT private key",
zap.String("private-key-path", jwtPrivKeyPath),
zap.Error(err),
)
} else {
plog.Errorf("failed to read private key (%s) for jwt: %s", jwtPrivKeyPath, err)
}
return nil, err
}
t.signKey, err = jwt.ParseRSAPrivateKeyFromPEM(signBytes)
if err != nil {
plog.Errorf("failed to parse private key (%s): %s", jwtPrivKeyPath, err)
if lg != nil {
lg.Warn(
"failed to parse JWT private key",
zap.String("private-key-path", jwtPrivKeyPath),
zap.Error(err),
)
} else {
plog.Errorf("failed to parse private key (%s): %s", jwtPrivKeyPath, err)
}
return nil, err
}

Expand Down
14 changes: 8 additions & 6 deletions auth/jwt_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ package auth
import (
"context"
"testing"

"go.uber.org/zap"
)

const (
Expand All @@ -30,7 +32,7 @@ func TestJWTInfo(t *testing.T) {
"priv-key": jwtPrivKey,
"sign-method": "RS256",
}
jwt, err := newTokenProviderJWT(opts)
jwt, err := newTokenProviderJWT(zap.NewExample(), opts)
if err != nil {
t.Fatal(err)
}
Expand Down Expand Up @@ -59,35 +61,35 @@ func TestJWTBad(t *testing.T) {
}
// private key instead of public key
opts["pub-key"] = jwtPrivKey
if _, err := newTokenProviderJWT(opts); err == nil {
if _, err := newTokenProviderJWT(zap.NewExample(), opts); err == nil {
t.Fatalf("expected failure on missing public key")
}
opts["pub-key"] = jwtPubKey

// public key instead of private key
opts["priv-key"] = jwtPubKey
if _, err := newTokenProviderJWT(opts); err == nil {
if _, err := newTokenProviderJWT(zap.NewExample(), opts); err == nil {
t.Fatalf("expected failure on missing public key")
}
opts["priv-key"] = jwtPrivKey

// missing signing option
delete(opts, "sign-method")
if _, err := newTokenProviderJWT(opts); err == nil {
if _, err := newTokenProviderJWT(zap.NewExample(), opts); err == nil {
t.Fatal("expected error on missing option")
}
opts["sign-method"] = "RS256"

// bad file for pubkey
opts["pub-key"] = "whatever"
if _, err := newTokenProviderJWT(opts); err == nil {
if _, err := newTokenProviderJWT(zap.NewExample(), opts); err == nil {
t.Fatalf("expected failure on missing public key")
}
opts["pub-key"] = jwtPubKey

// bad file for private key
opts["priv-key"] = "whatever"
if _, err := newTokenProviderJWT(opts); err == nil {
if _, err := newTokenProviderJWT(zap.NewExample(), opts); err == nil {
t.Fatalf("expeceted failure on missing private key")
}
opts["priv-key"] = jwtPrivKey
Expand Down
42 changes: 31 additions & 11 deletions auth/range_perm_cache.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,13 @@ import (
"github.com/coreos/etcd/auth/authpb"
"github.com/coreos/etcd/mvcc/backend"
"github.com/coreos/etcd/pkg/adt"

"go.uber.org/zap"
)

func getMergedPerms(tx backend.BatchTx, userName string) *unifiedRangePermissions {
user := getUser(tx, userName)
func getMergedPerms(lg *zap.Logger, tx backend.BatchTx, userName string) *unifiedRangePermissions {
user := getUser(lg, tx, userName)
if user == nil {
plog.Errorf("invalid user name %s", userName)
return nil
}

Expand Down Expand Up @@ -70,7 +71,11 @@ func getMergedPerms(tx backend.BatchTx, userName string) *unifiedRangePermission
}
}

func checkKeyInterval(cachedPerms *unifiedRangePermissions, key, rangeEnd []byte, permtyp authpb.Permission_Type) bool {
func checkKeyInterval(
lg *zap.Logger,
cachedPerms *unifiedRangePermissions,
key, rangeEnd []byte,
permtyp authpb.Permission_Type) bool {
if len(rangeEnd) == 1 && rangeEnd[0] == 0 {
rangeEnd = nil
}
Expand All @@ -82,20 +87,28 @@ func checkKeyInterval(cachedPerms *unifiedRangePermissions, key, rangeEnd []byte
case authpb.WRITE:
return cachedPerms.writePerms.Contains(ivl)
default:
plog.Panicf("unknown auth type: %v", permtyp)
if lg != nil {
lg.Panic("unknown auth type", zap.String("auth-type", permtyp.String()))
} else {
plog.Panicf("unknown auth type: %v", permtyp)
}
}
return false
}

func checkKeyPoint(cachedPerms *unifiedRangePermissions, key []byte, permtyp authpb.Permission_Type) bool {
func checkKeyPoint(lg *zap.Logger, cachedPerms *unifiedRangePermissions, key []byte, permtyp authpb.Permission_Type) bool {
pt := adt.NewBytesAffinePoint(key)
switch permtyp {
case authpb.READ:
return cachedPerms.readPerms.Intersects(pt)
case authpb.WRITE:
return cachedPerms.writePerms.Intersects(pt)
default:
plog.Panicf("unknown auth type: %v", permtyp)
if lg != nil {
lg.Panic("unknown auth type", zap.String("auth-type", permtyp.String()))
} else {
plog.Panicf("unknown auth type: %v", permtyp)
}
}
return false
}
Expand All @@ -104,19 +117,26 @@ func (as *authStore) isRangeOpPermitted(tx backend.BatchTx, userName string, key
// assumption: tx is Lock()ed
_, ok := as.rangePermCache[userName]
if !ok {
perms := getMergedPerms(tx, userName)
perms := getMergedPerms(as.lg, tx, userName)
if perms == nil {
plog.Errorf("failed to create a unified permission of user %s", userName)
if as.lg != nil {
as.lg.Warn(
"failed to create a merged permission",
zap.String("user-name", userName),
)
} else {
plog.Errorf("failed to create a unified permission of user %s", userName)
}
return false
}
as.rangePermCache[userName] = perms
}

if len(rangeEnd) == 0 {
return checkKeyPoint(as.rangePermCache[userName], key, permtyp)
return checkKeyPoint(as.lg, as.rangePermCache[userName], key, permtyp)
}

return checkKeyInterval(as.rangePermCache[userName], key, rangeEnd, permtyp)
return checkKeyInterval(as.lg, as.rangePermCache[userName], key, rangeEnd, permtyp)
}

func (as *authStore) clearCachedPerm() {
Expand Down
4 changes: 3 additions & 1 deletion auth/range_perm_cache_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import (

"github.com/coreos/etcd/auth/authpb"
"github.com/coreos/etcd/pkg/adt"

"go.uber.org/zap"
)

func TestRangePermission(t *testing.T) {
Expand Down Expand Up @@ -51,7 +53,7 @@ func TestRangePermission(t *testing.T) {
readPerms.Insert(p, struct{}{})
}

result := checkKeyInterval(&unifiedRangePermissions{readPerms: readPerms}, tt.begin, tt.end, authpb.READ)
result := checkKeyInterval(zap.NewExample(), &unifiedRangePermissions{readPerms: readPerms}, tt.begin, tt.end, authpb.READ)
if result != tt.want {
t.Errorf("#%d: result=%t, want=%t", i, result, tt.want)
}
Expand Down

0 comments on commit 4bab1e1

Please sign in to comment.