From 4007e7f61a2ced7ebc4eb5d0d4eae382a32059af Mon Sep 17 00:00:00 2001 From: Hidetake Iwata Date: Tue, 29 Oct 2019 09:55:29 +0900 Subject: [PATCH] Refactor: extract jwtdecoder package (#174) --- pkg/adaptors/{oidc => jwtdecoder}/decoder.go | 31 ++++++++---- .../{oidc => jwtdecoder}/decoder_test.go | 12 ++--- .../mock_jwtdecoder/mock_jwtdecoder.go | 49 +++++++++++++++++++ .../{oidc => jwtdecoder}/testdata/Makefile | 0 .../{oidc => jwtdecoder}/testdata/jws.key | 0 pkg/adaptors/oidc/client.go | 13 +++++ pkg/adaptors/oidc/mock_oidc/mock_oidc.go | 40 +-------------- pkg/adaptors/oidc/oidc.go | 4 +- pkg/di/di.go | 3 ++ pkg/di/wire_gen.go | 9 ++-- pkg/usecases/authentication/authentication.go | 17 ++++--- .../authentication/authentication_test.go | 38 +++++++------- 12 files changed, 128 insertions(+), 88 deletions(-) rename pkg/adaptors/{oidc => jwtdecoder}/decoder.go (62%) rename pkg/adaptors/{oidc => jwtdecoder}/decoder_test.go (87%) create mode 100644 pkg/adaptors/jwtdecoder/mock_jwtdecoder/mock_jwtdecoder.go rename pkg/adaptors/{oidc => jwtdecoder}/testdata/Makefile (100%) rename pkg/adaptors/{oidc => jwtdecoder}/testdata/jws.key (100%) diff --git a/pkg/adaptors/oidc/decoder.go b/pkg/adaptors/jwtdecoder/decoder.go similarity index 62% rename from pkg/adaptors/oidc/decoder.go rename to pkg/adaptors/jwtdecoder/decoder.go index 143706af..4f1bebd0 100644 --- a/pkg/adaptors/oidc/decoder.go +++ b/pkg/adaptors/jwtdecoder/decoder.go @@ -1,4 +1,5 @@ -package oidc +// Package jwtdecoder provides decoding a JWT. +package jwtdecoder import ( "bytes" @@ -8,25 +9,35 @@ import ( "time" "github.com/dgrijalva/jwt-go" + "github.com/google/wire" "golang.org/x/xerrors" ) -type DecoderInterface interface { - DecodeIDToken(t string) (*DecodedIDToken, error) +//go:generate mockgen -destination mock_jwtdecoder/mock_jwtdecoder.go github.com/int128/kubelogin/pkg/adaptors/jwtdecoder Interface + +// Set provides an implementation and interface. +var Set = wire.NewSet( + wire.Struct(new(Decoder), "*"), + wire.Bind(new(Interface), new(*Decoder)), +) + +type Interface interface { + Decode(s string) (*Claims, error) } -type DecodedIDToken struct { +// Claims represents claims of a token. +type Claims struct { Subject string Expiry time.Time - Claims map[string]string // string representation of claims for logging + Pretty map[string]string // string representation for debug and logging } type Decoder struct{} -// DecodeIDToken returns the claims of the ID token. +// Decode returns the claims of the JWT. // Note that this method does not verify the signature and always trust it. -func (d *Decoder) DecodeIDToken(t string) (*DecodedIDToken, error) { - parts := strings.Split(t, ".") +func (d *Decoder) Decode(s string) (*Claims, error) { + parts := strings.Split(s, ".") if len(parts) != 3 { return nil, xerrors.Errorf("token contains an invalid number of segments") } @@ -42,10 +53,10 @@ func (d *Decoder) DecodeIDToken(t string) (*DecodedIDToken, error) { if err := json.NewDecoder(bytes.NewBuffer(b)).Decode(&rawClaims); err != nil { return nil, xerrors.Errorf("could not decode the json of token: %w", err) } - return &DecodedIDToken{ + return &Claims{ Subject: claims.Subject, Expiry: time.Unix(claims.ExpiresAt, 0), - Claims: dumpRawClaims(rawClaims), + Pretty: dumpRawClaims(rawClaims), }, nil } diff --git a/pkg/adaptors/oidc/decoder_test.go b/pkg/adaptors/jwtdecoder/decoder_test.go similarity index 87% rename from pkg/adaptors/oidc/decoder_test.go rename to pkg/adaptors/jwtdecoder/decoder_test.go index a2d7686e..a76a45dd 100644 --- a/pkg/adaptors/oidc/decoder_test.go +++ b/pkg/adaptors/jwtdecoder/decoder_test.go @@ -1,4 +1,4 @@ -package oidc +package jwtdecoder import ( "crypto/rsa" @@ -11,23 +11,23 @@ import ( "github.com/dgrijalva/jwt-go" ) -func TestDecoder_DecodeIDToken(t *testing.T) { +func TestDecoder_Decode(t *testing.T) { var decoder Decoder t.Run("ValidToken", func(t *testing.T) { expiry := time.Now().Round(time.Second) idToken := newIDToken(t, "https://issuer.example.com", expiry) - decodedToken, err := decoder.DecodeIDToken(idToken) + decodedToken, err := decoder.Decode(idToken) if err != nil { - t.Fatalf("DecodeIDToken error: %s", err) + t.Fatalf("Decode error: %s", err) } if decodedToken.Expiry != expiry { t.Errorf("Expiry wants %s but %s", expiry, decodedToken.Expiry) } - t.Logf("Claims=%+v", decodedToken.Claims) + t.Logf("Pretty=%+v", decodedToken.Pretty) }) t.Run("InvalidToken", func(t *testing.T) { - decodedToken, err := decoder.DecodeIDToken("HEADER.INVALID_TOKEN.SIGNATURE") + decodedToken, err := decoder.Decode("HEADER.INVALID_TOKEN.SIGNATURE") if err == nil { t.Errorf("error wants non-nil but nil") } else { diff --git a/pkg/adaptors/jwtdecoder/mock_jwtdecoder/mock_jwtdecoder.go b/pkg/adaptors/jwtdecoder/mock_jwtdecoder/mock_jwtdecoder.go new file mode 100644 index 00000000..e7df87c3 --- /dev/null +++ b/pkg/adaptors/jwtdecoder/mock_jwtdecoder/mock_jwtdecoder.go @@ -0,0 +1,49 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/int128/kubelogin/pkg/adaptors/jwtdecoder (interfaces: Interface) + +// Package mock_jwtdecoder is a generated GoMock package. +package mock_jwtdecoder + +import ( + gomock "github.com/golang/mock/gomock" + jwtdecoder "github.com/int128/kubelogin/pkg/adaptors/jwtdecoder" + reflect "reflect" +) + +// MockInterface is a mock of Interface interface +type MockInterface struct { + ctrl *gomock.Controller + recorder *MockInterfaceMockRecorder +} + +// MockInterfaceMockRecorder is the mock recorder for MockInterface +type MockInterfaceMockRecorder struct { + mock *MockInterface +} + +// NewMockInterface creates a new mock instance +func NewMockInterface(ctrl *gomock.Controller) *MockInterface { + mock := &MockInterface{ctrl: ctrl} + mock.recorder = &MockInterfaceMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use +func (m *MockInterface) EXPECT() *MockInterfaceMockRecorder { + return m.recorder +} + +// Decode mocks base method +func (m *MockInterface) Decode(arg0 string) (*jwtdecoder.Claims, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Decode", arg0) + ret0, _ := ret[0].(*jwtdecoder.Claims) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Decode indicates an expected call of Decode +func (mr *MockInterfaceMockRecorder) Decode(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Decode", reflect.TypeOf((*MockInterface)(nil).Decode), arg0) +} diff --git a/pkg/adaptors/oidc/testdata/Makefile b/pkg/adaptors/jwtdecoder/testdata/Makefile similarity index 100% rename from pkg/adaptors/oidc/testdata/Makefile rename to pkg/adaptors/jwtdecoder/testdata/Makefile diff --git a/pkg/adaptors/oidc/testdata/jws.key b/pkg/adaptors/jwtdecoder/testdata/jws.key similarity index 100% rename from pkg/adaptors/oidc/testdata/jws.key rename to pkg/adaptors/jwtdecoder/testdata/jws.key diff --git a/pkg/adaptors/oidc/client.go b/pkg/adaptors/oidc/client.go index a09404bc..0d16503c 100644 --- a/pkg/adaptors/oidc/client.go +++ b/pkg/adaptors/oidc/client.go @@ -160,3 +160,16 @@ func dumpClaims(token *oidc.IDToken) (map[string]string, error) { err := token.Claims(&rawClaims) return dumpRawClaims(rawClaims), err } + +func dumpRawClaims(rawClaims map[string]interface{}) map[string]string { + claims := make(map[string]string) + for k, v := range rawClaims { + switch v := v.(type) { + case float64: + claims[k] = fmt.Sprintf("%.f", v) + default: + claims[k] = fmt.Sprintf("%v", v) + } + } + return claims +} diff --git a/pkg/adaptors/oidc/mock_oidc/mock_oidc.go b/pkg/adaptors/oidc/mock_oidc/mock_oidc.go index 65c5f6ac..d8cdb35d 100644 --- a/pkg/adaptors/oidc/mock_oidc/mock_oidc.go +++ b/pkg/adaptors/oidc/mock_oidc/mock_oidc.go @@ -1,5 +1,5 @@ // Code generated by MockGen. DO NOT EDIT. -// Source: github.com/int128/kubelogin/pkg/adaptors/oidc (interfaces: FactoryInterface,Interface,DecoderInterface) +// Source: github.com/int128/kubelogin/pkg/adaptors/oidc (interfaces: FactoryInterface,Interface) // Package mock_oidc is a generated GoMock package. package mock_oidc @@ -116,41 +116,3 @@ func (mr *MockInterfaceMockRecorder) Refresh(arg0, arg1 interface{}) *gomock.Cal mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Refresh", reflect.TypeOf((*MockInterface)(nil).Refresh), arg0, arg1) } - -// MockDecoderInterface is a mock of DecoderInterface interface -type MockDecoderInterface struct { - ctrl *gomock.Controller - recorder *MockDecoderInterfaceMockRecorder -} - -// MockDecoderInterfaceMockRecorder is the mock recorder for MockDecoderInterface -type MockDecoderInterfaceMockRecorder struct { - mock *MockDecoderInterface -} - -// NewMockDecoderInterface creates a new mock instance -func NewMockDecoderInterface(ctrl *gomock.Controller) *MockDecoderInterface { - mock := &MockDecoderInterface{ctrl: ctrl} - mock.recorder = &MockDecoderInterfaceMockRecorder{mock} - return mock -} - -// EXPECT returns an object that allows the caller to indicate expected use -func (m *MockDecoderInterface) EXPECT() *MockDecoderInterfaceMockRecorder { - return m.recorder -} - -// DecodeIDToken mocks base method -func (m *MockDecoderInterface) DecodeIDToken(arg0 string) (*oidc.DecodedIDToken, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "DecodeIDToken", arg0) - ret0, _ := ret[0].(*oidc.DecodedIDToken) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// DecodeIDToken indicates an expected call of DecodeIDToken -func (mr *MockDecoderInterfaceMockRecorder) DecodeIDToken(arg0 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DecodeIDToken", reflect.TypeOf((*MockDecoderInterface)(nil).DecodeIDToken), arg0) -} diff --git a/pkg/adaptors/oidc/oidc.go b/pkg/adaptors/oidc/oidc.go index 5d71dd1b..d8183938 100644 --- a/pkg/adaptors/oidc/oidc.go +++ b/pkg/adaptors/oidc/oidc.go @@ -4,12 +4,10 @@ import ( "github.com/google/wire" ) -//go:generate mockgen -destination mock_oidc/mock_oidc.go github.com/int128/kubelogin/pkg/adaptors/oidc FactoryInterface,Interface,DecoderInterface +//go:generate mockgen -destination mock_oidc/mock_oidc.go github.com/int128/kubelogin/pkg/adaptors/oidc FactoryInterface,Interface // Set provides an implementation and interface for OIDC. var Set = wire.NewSet( wire.Struct(new(Factory), "*"), wire.Bind(new(FactoryInterface), new(*Factory)), - wire.Struct(new(Decoder)), - wire.Bind(new(DecoderInterface), new(*Decoder)), ) diff --git a/pkg/di/di.go b/pkg/di/di.go index 2e3c26b2..b36de5de 100644 --- a/pkg/di/di.go +++ b/pkg/di/di.go @@ -9,6 +9,7 @@ import ( "github.com/int128/kubelogin/pkg/adaptors/cmd" credentialPluginAdaptor "github.com/int128/kubelogin/pkg/adaptors/credentialplugin" "github.com/int128/kubelogin/pkg/adaptors/env" + "github.com/int128/kubelogin/pkg/adaptors/jwtdecoder" "github.com/int128/kubelogin/pkg/adaptors/kubeconfig" "github.com/int128/kubelogin/pkg/adaptors/logger" "github.com/int128/kubelogin/pkg/adaptors/oidc" @@ -36,6 +37,7 @@ func NewCmd() cmd.Interface { tokencache.Set, credentialPluginAdaptor.Set, oidc.Set, + jwtdecoder.Set, certpool.Set, logger.Set, ) @@ -55,6 +57,7 @@ func NewCmdForHeadless(logger.Interface, authentication.LocalServerReadyFunc, cr kubeconfig.Set, tokencache.Set, oidc.Set, + jwtdecoder.Set, certpool.Set, ) return nil diff --git a/pkg/di/wire_gen.go b/pkg/di/wire_gen.go index 52a5380c..4948edeb 100644 --- a/pkg/di/wire_gen.go +++ b/pkg/di/wire_gen.go @@ -10,6 +10,7 @@ import ( "github.com/int128/kubelogin/pkg/adaptors/cmd" "github.com/int128/kubelogin/pkg/adaptors/credentialplugin" "github.com/int128/kubelogin/pkg/adaptors/env" + "github.com/int128/kubelogin/pkg/adaptors/jwtdecoder" "github.com/int128/kubelogin/pkg/adaptors/kubeconfig" "github.com/int128/kubelogin/pkg/adaptors/logger" "github.com/int128/kubelogin/pkg/adaptors/oidc" @@ -27,12 +28,12 @@ func NewCmd() cmd.Interface { factory := &oidc.Factory{ Logger: loggerInterface, } - decoder := &oidc.Decoder{} + decoder := &jwtdecoder.Decoder{} envEnv := &env.Env{} localServerReadyFunc := _wireLocalServerReadyFuncValue authenticationAuthentication := &authentication.Authentication{ OIDCFactory: factory, - OIDCDecoder: decoder, + JWTDecoder: decoder, Env: envEnv, Logger: loggerInterface, LocalServerReadyFunc: localServerReadyFunc, @@ -89,11 +90,11 @@ func NewCmdForHeadless(loggerInterface logger.Interface, localServerReadyFunc au factory := &oidc.Factory{ Logger: loggerInterface, } - decoder := &oidc.Decoder{} + decoder := &jwtdecoder.Decoder{} envEnv := &env.Env{} authenticationAuthentication := &authentication.Authentication{ OIDCFactory: factory, - OIDCDecoder: decoder, + JWTDecoder: decoder, Env: envEnv, Logger: loggerInterface, LocalServerReadyFunc: localServerReadyFunc, diff --git a/pkg/usecases/authentication/authentication.go b/pkg/usecases/authentication/authentication.go index 5feb2283..33dc70c4 100644 --- a/pkg/usecases/authentication/authentication.go +++ b/pkg/usecases/authentication/authentication.go @@ -7,6 +7,7 @@ import ( "github.com/google/wire" "github.com/int128/kubelogin/pkg/adaptors/certpool" "github.com/int128/kubelogin/pkg/adaptors/env" + "github.com/int128/kubelogin/pkg/adaptors/jwtdecoder" "github.com/int128/kubelogin/pkg/adaptors/logger" "github.com/int128/kubelogin/pkg/adaptors/oidc" "golang.org/x/sync/errgroup" @@ -74,7 +75,7 @@ const passwordPrompt = "Password: " // type Authentication struct { OIDCFactory oidc.FactoryInterface - OIDCDecoder oidc.DecoderInterface + JWTDecoder jwtdecoder.Interface Env env.Interface Logger logger.Interface LocalServerReadyFunc LocalServerReadyFunc // only for e2e tests @@ -86,22 +87,22 @@ func (u *Authentication) Do(ctx context.Context, in Input) (*Output, error) { // Skip verification of the token to reduce time of a discovery request. // Here it trusts the signature and claims and checks only expiration, // because the token has been verified before caching. - token, err := u.OIDCDecoder.DecodeIDToken(in.IDToken) + claims, err := u.JWTDecoder.Decode(in.IDToken) if err != nil { return nil, xerrors.Errorf("invalid token and you need to remove the cache: %w", err) } - if token.Expiry.After(time.Now()) { //TODO: inject time service - u.Logger.V(1).Infof("you already have a valid token until %s", token.Expiry) + if claims.Expiry.After(time.Now()) { //TODO: inject time service + u.Logger.V(1).Infof("you already have a valid token until %s", claims.Expiry) return &Output{ AlreadyHasValidIDToken: true, IDToken: in.IDToken, RefreshToken: in.RefreshToken, - IDTokenSubject: token.Subject, - IDTokenExpiry: token.Expiry, - IDTokenClaims: token.Claims, + IDTokenSubject: claims.Subject, + IDTokenExpiry: claims.Expiry, + IDTokenClaims: claims.Pretty, }, nil } - u.Logger.V(1).Infof("you have an expired token at %s", token.Expiry) + u.Logger.V(1).Infof("you have an expired token at %s", claims.Expiry) } u.Logger.V(1).Infof("initializing an OIDCFactory client") diff --git a/pkg/usecases/authentication/authentication_test.go b/pkg/usecases/authentication/authentication_test.go index 66ef71dd..d153ce50 100644 --- a/pkg/usecases/authentication/authentication_test.go +++ b/pkg/usecases/authentication/authentication_test.go @@ -9,6 +9,8 @@ import ( "github.com/golang/mock/gomock" "github.com/int128/kubelogin/pkg/adaptors/certpool/mock_certpool" "github.com/int128/kubelogin/pkg/adaptors/env/mock_env" + "github.com/int128/kubelogin/pkg/adaptors/jwtdecoder" + "github.com/int128/kubelogin/pkg/adaptors/jwtdecoder/mock_jwtdecoder" "github.com/int128/kubelogin/pkg/adaptors/logger/mock_logger" "github.com/int128/kubelogin/pkg/adaptors/oidc" "github.com/int128/kubelogin/pkg/adaptors/oidc/mock_oidc" @@ -287,17 +289,17 @@ func TestAuthentication_Do(t *testing.T) { ClientSecret: "YOUR_CLIENT_SECRET", IDToken: "VALID_ID_TOKEN", } - mockOIDCDecoder := mock_oidc.NewMockDecoderInterface(ctrl) - mockOIDCDecoder.EXPECT(). - DecodeIDToken("VALID_ID_TOKEN"). - Return(&oidc.DecodedIDToken{ + mockDecoder := mock_jwtdecoder.NewMockInterface(ctrl) + mockDecoder.EXPECT(). + Decode("VALID_ID_TOKEN"). + Return(&jwtdecoder.Claims{ Subject: "YOUR_SUBJECT", Expiry: futureTime, - Claims: dummyTokenClaims, + Pretty: dummyTokenClaims, }, nil) u := Authentication{ OIDCFactory: mock_oidc.NewMockFactoryInterface(ctrl), - OIDCDecoder: mockOIDCDecoder, + JWTDecoder: mockDecoder, Logger: mock_logger.New(t), } out, err := u.Do(ctx, in) @@ -328,13 +330,13 @@ func TestAuthentication_Do(t *testing.T) { IDToken: "EXPIRED_ID_TOKEN", RefreshToken: "VALID_REFRESH_TOKEN", } - mockOIDCDecoder := mock_oidc.NewMockDecoderInterface(ctrl) - mockOIDCDecoder.EXPECT(). - DecodeIDToken("EXPIRED_ID_TOKEN"). - Return(&oidc.DecodedIDToken{ + mockDecoder := mock_jwtdecoder.NewMockInterface(ctrl) + mockDecoder.EXPECT(). + Decode("EXPIRED_ID_TOKEN"). + Return(&jwtdecoder.Claims{ Subject: "YOUR_SUBJECT", Expiry: pastTime, - Claims: dummyTokenClaims, + Pretty: dummyTokenClaims, }, nil) mockOIDCClient := mock_oidc.NewMockInterface(ctrl) mockOIDCClient.EXPECT(). @@ -356,7 +358,7 @@ func TestAuthentication_Do(t *testing.T) { Return(mockOIDCClient, nil) u := Authentication{ OIDCFactory: mockOIDCFactory, - OIDCDecoder: mockOIDCDecoder, + JWTDecoder: mockDecoder, Logger: mock_logger.New(t), } out, err := u.Do(ctx, in) @@ -389,13 +391,13 @@ func TestAuthentication_Do(t *testing.T) { IDToken: "EXPIRED_ID_TOKEN", RefreshToken: "EXPIRED_REFRESH_TOKEN", } - mockOIDCDecoder := mock_oidc.NewMockDecoderInterface(ctrl) - mockOIDCDecoder.EXPECT(). - DecodeIDToken("EXPIRED_ID_TOKEN"). - Return(&oidc.DecodedIDToken{ + mockDecoder := mock_jwtdecoder.NewMockInterface(ctrl) + mockDecoder.EXPECT(). + Decode("EXPIRED_ID_TOKEN"). + Return(&jwtdecoder.Claims{ Subject: "YOUR_SUBJECT", Expiry: pastTime, - Claims: dummyTokenClaims, + Pretty: dummyTokenClaims, }, nil) mockOIDCClient := mock_oidc.NewMockInterface(ctrl) mockOIDCClient.EXPECT(). @@ -423,7 +425,7 @@ func TestAuthentication_Do(t *testing.T) { Return(mockOIDCClient, nil) u := Authentication{ OIDCFactory: mockOIDCFactory, - OIDCDecoder: mockOIDCDecoder, + JWTDecoder: mockDecoder, Logger: mock_logger.New(t), } out, err := u.Do(ctx, in)