From d57fea33bf359c8bee5675bff270b9b453e2db41 Mon Sep 17 00:00:00 2001 From: Hoeg Date: Fri, 19 Jan 2024 12:31:44 +0100 Subject: [PATCH 1/3] prep for tests --- internal/http/authenticator.go | 61 ++++++++++++++++++++--------- internal/http/authenticator_test.go | 57 +++++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 18 deletions(-) create mode 100644 internal/http/authenticator_test.go diff --git a/internal/http/authenticator.go b/internal/http/authenticator.go index 0b5efcce..5888e882 100644 --- a/internal/http/authenticator.go +++ b/internal/http/authenticator.go @@ -19,9 +19,22 @@ var ( ErrTokenExpired = errors.New("oauth2: token expired and refresh token is not set") ) +type TokenStore interface { + storeAccessToken(token *oauth2.Token) error + readAccessToken() (*oauth2.Token, error) +} + +type DeviceAuthenticator interface { + DeviceAuth(ctx context.Context, opts ...oauth2.AuthCodeOption) (*oauth2.DeviceAuthResponse, error) + DeviceAccessToken(ctx context.Context, da *oauth2.DeviceAuthResponse, opts ...oauth2.AuthCodeOption) (*oauth2.Token, error) + Client(ctx context.Context, t *oauth2.Token) *http.Client +} + type UserAuthenticator struct { - conf *oauth2.Config - autoLogin bool + conf DeviceAuthenticator + ts TokenStore + autoLogin bool + popBrowser bool } func NewUserAuthenticator(clientID, idpURL string, autoLogin bool) UserAuthenticator { @@ -34,8 +47,10 @@ func NewUserAuthenticator(clientID, idpURL string, autoLogin bool) UserAuthentic Scopes: []string{"openid profile"}, } return UserAuthenticator{ - conf: conf, - autoLogin: autoLogin, + conf: conf, + ts: newTokenStore(), + autoLogin: autoLogin, + popBrowser: true, } } @@ -44,7 +59,7 @@ func (g *UserAuthenticator) Login(ctx context.Context) error { if err != nil { return err } - return storeAccessToken(token) + return g.ts.storeAccessToken(token) } func (g *UserAuthenticator) login(ctx context.Context) (*oauth2.Token, error) { @@ -53,23 +68,24 @@ func (g *UserAuthenticator) login(ctx context.Context) (*oauth2.Token, error) { return nil, err } fmt.Printf("please enter code %s at %s\n", response.UserCode, response.VerificationURIComplete) - err = browser.OpenURL(response.VerificationURIComplete) - if err != nil { - return nil, err + if g.popBrowser { + err = browser.OpenURL(response.VerificationURIComplete) + if err != nil { + return nil, err + } } - return g.conf.DeviceAccessToken(ctx, response) } func (g *UserAuthenticator) Access(ctx context.Context) (*http.Client, error) { - token, err := readAccessToken() + token, err := g.ts.readAccessToken() if err != nil { if g.autoLogin { token, err = g.login(ctx) if err != nil { return nil, fmt.Errorf("auto login failed: %w: %w", ErrLoginRequired, err) } - err = storeAccessToken(token) + err = g.ts.storeAccessToken(token) if err != nil { return nil, err } @@ -82,7 +98,7 @@ func (g *UserAuthenticator) Access(ctx context.Context) (*http.Client, error) { if err != nil { return nil, fmt.Errorf("auto login failed: %w: %w", ErrLoginRequired, err) } - err = storeAccessToken(token) + err = g.ts.storeAccessToken(token) if err != nil { return nil, err } @@ -96,8 +112,18 @@ func tokenFilePath() string { return filepath.Join(os.Getenv("HOME"), tokenFile) } -func readAccessToken() (*oauth2.Token, error) { - data, err := os.ReadFile(tokenFilePath()) +type tokenStore struct { + tokenFilePath string +} + +func newTokenStore() *tokenStore { + return &tokenStore{ + tokenFilePath: tokenFilePath(), + } +} + +func (ts *tokenStore) readAccessToken() (*oauth2.Token, error) { + data, err := os.ReadFile(ts.tokenFilePath) if err != nil { return nil, err } @@ -109,17 +135,16 @@ func readAccessToken() (*oauth2.Token, error) { return &token, nil } -func storeAccessToken(token *oauth2.Token) error { +func (ts *tokenStore) storeAccessToken(token *oauth2.Token) error { accessToken, err := json.Marshal(token) if err != nil { return err } - p := tokenFilePath() - dir := filepath.Dir(p) + dir := filepath.Dir(ts.tokenFilePath) if err := os.MkdirAll(dir, 0700); err != nil { return err } - f, err := os.Create(p) + f, err := os.Create(ts.tokenFilePath) if err != nil { return err } diff --git a/internal/http/authenticator_test.go b/internal/http/authenticator_test.go new file mode 100644 index 00000000..f595f3d4 --- /dev/null +++ b/internal/http/authenticator_test.go @@ -0,0 +1,57 @@ +package http + +import ( + "context" + "net/http" + "testing" + + "golang.org/x/oauth2" +) + +func TestAccess(t *testing.T) { + ua := UserAuthenticator{ + conf: &HttpAuth{ + DeviceResp: &oauth2.DeviceAuthResponse{ + UserCode: "YOLO", + VerificationURIComplete: "localhost", + }, + }, + ts: &DummyStore{}, + autoLogin: true, + popBrowser: false, + } + + client, err := ua.Access(context.Background()) + if err != nil { + t.Fatal(err) + } + if client == nil { + t.Fatal("client is nil") + } +} + +type HttpAuth struct { + DeviceResp *oauth2.DeviceAuthResponse +} + +func (ha *HttpAuth) DeviceAuth(ctx context.Context, opts ...oauth2.AuthCodeOption) (*oauth2.DeviceAuthResponse, error) { + return ha.DeviceResp, nil +} + +func (ha *HttpAuth) DeviceAccessToken(ctx context.Context, da *oauth2.DeviceAuthResponse, opts ...oauth2.AuthCodeOption) (*oauth2.Token, error) { + return nil, nil +} + +func (ha *HttpAuth) Client(ctx context.Context, t *oauth2.Token) *http.Client { + return http.DefaultClient +} + +type DummyStore struct { +} + +func (ds *DummyStore) storeAccessToken(token *oauth2.Token) error { + return nil +} +func (ds *DummyStore) readAccessToken() (*oauth2.Token, error) { + return nil, nil +} From 4680e9ae5e7f23fd0588c1d74f1cd400fbd6ebee Mon Sep 17 00:00:00 2001 From: Hoeg Date: Fri, 19 Jan 2024 12:57:14 +0100 Subject: [PATCH 2/3] login called on action --- internal/http/authenticator_test.go | 40 +++++++++++++++++++++-------- 1 file changed, 30 insertions(+), 10 deletions(-) diff --git a/internal/http/authenticator_test.go b/internal/http/authenticator_test.go index f595f3d4..d4673a5c 100644 --- a/internal/http/authenticator_test.go +++ b/internal/http/authenticator_test.go @@ -2,21 +2,26 @@ package http import ( "context" + "fmt" "net/http" "testing" "golang.org/x/oauth2" ) -func TestAccess(t *testing.T) { - ua := UserAuthenticator{ - conf: &HttpAuth{ - DeviceResp: &oauth2.DeviceAuthResponse{ - UserCode: "YOLO", - VerificationURIComplete: "localhost", - }, +func TestAccessCallsLoginAndStoresToken(t *testing.T) { + httpAuth := HttpAuth{ + DeviceResp: &oauth2.DeviceAuthResponse{ + UserCode: "YOLO", + VerificationURIComplete: "localhost", }, - ts: &DummyStore{}, + } + dummyStore := DummyStore{ + token: nil, + } + ua := UserAuthenticator{ + conf: &httpAuth, + ts: &dummyStore, autoLogin: true, popBrowser: false, } @@ -28,10 +33,17 @@ func TestAccess(t *testing.T) { if client == nil { t.Fatal("client is nil") } + if dummyStore.tokenRead == false { + t.Fatal("expected a read of the token") + } + if dummyStore.tokenStored == false { + t.Fatal("expected the token to be stored") + } } type HttpAuth struct { DeviceResp *oauth2.DeviceAuthResponse + token *oauth2.Token } func (ha *HttpAuth) DeviceAuth(ctx context.Context, opts ...oauth2.AuthCodeOption) (*oauth2.DeviceAuthResponse, error) { @@ -39,7 +51,7 @@ func (ha *HttpAuth) DeviceAuth(ctx context.Context, opts ...oauth2.AuthCodeOptio } func (ha *HttpAuth) DeviceAccessToken(ctx context.Context, da *oauth2.DeviceAuthResponse, opts ...oauth2.AuthCodeOption) (*oauth2.Token, error) { - return nil, nil + return ha.token, nil } func (ha *HttpAuth) Client(ctx context.Context, t *oauth2.Token) *http.Client { @@ -47,11 +59,19 @@ func (ha *HttpAuth) Client(ctx context.Context, t *oauth2.Token) *http.Client { } type DummyStore struct { + token *oauth2.Token + tokenStored bool + tokenRead bool } func (ds *DummyStore) storeAccessToken(token *oauth2.Token) error { + ds.tokenStored = true return nil } func (ds *DummyStore) readAccessToken() (*oauth2.Token, error) { - return nil, nil + ds.tokenRead = true + if ds.token != nil { + return nil, fmt.Errorf("no token found") + } + return ds.token, nil } From b905964ce5b365096746e7a027ab8bd5df9bd7f9 Mon Sep 17 00:00:00 2001 From: Hoeg Date: Fri, 19 Jan 2024 13:09:17 +0100 Subject: [PATCH 3/3] added some tests --- internal/http/authenticator_test.go | 83 +++++++++++++++++++++++++++-- 1 file changed, 79 insertions(+), 4 deletions(-) diff --git a/internal/http/authenticator_test.go b/internal/http/authenticator_test.go index d4673a5c..fa435610 100644 --- a/internal/http/authenticator_test.go +++ b/internal/http/authenticator_test.go @@ -5,11 +5,12 @@ import ( "fmt" "net/http" "testing" + "time" "golang.org/x/oauth2" ) -func TestAccessCallsLoginAndStoresToken(t *testing.T) { +func TestAccessNoTokenCallsLoginAndStoresToken(t *testing.T) { httpAuth := HttpAuth{ DeviceResp: &oauth2.DeviceAuthResponse{ UserCode: "YOLO", @@ -41,12 +42,86 @@ func TestAccessCallsLoginAndStoresToken(t *testing.T) { } } +func TestAccessInvalidTokenCallsLoginAndStoresToken(t *testing.T) { + httpAuth := HttpAuth{ + DeviceResp: &oauth2.DeviceAuthResponse{ + UserCode: "YOLO", + VerificationURIComplete: "localhost", + }, + } + dummyStore := DummyStore{ + token: &oauth2.Token{ + AccessToken: "", + }, + } + ua := UserAuthenticator{ + conf: &httpAuth, + ts: &dummyStore, + autoLogin: true, + popBrowser: false, + } + + client, err := ua.Access(context.Background()) + if err != nil { + t.Fatal(err) + } + if client == nil { + t.Fatal("client is nil") + } + if dummyStore.tokenRead == false { + t.Fatal("expected a read of the token") + } + if dummyStore.tokenStored == false { + t.Fatal("expected the token to be stored") + } +} + +func TestAccessValidTokenNoLogin(t *testing.T) { + httpAuth := HttpAuth{ + DeviceResp: &oauth2.DeviceAuthResponse{ + UserCode: "YOLO", + VerificationURIComplete: "localhost", + }, + } + dummyStore := DummyStore{ + token: &oauth2.Token{ + AccessToken: "ACCESSTOKEN", + Expiry: time.Now().Add(1000 * time.Minute), + }, + } + ua := UserAuthenticator{ + conf: &httpAuth, + ts: &dummyStore, + autoLogin: true, + popBrowser: false, + } + + client, err := ua.Access(context.Background()) + if err != nil { + t.Fatal(err) + } + if client == nil { + t.Fatal("client is nil") + } + if !dummyStore.tokenRead { + t.Fatal("expected a read of the token") + } + if dummyStore.tokenStored { + t.Fatal("did not expect the token to be stored") + } + if httpAuth.loginCalled { + t.Fatal("did not expect login to be called") + } +} + type HttpAuth struct { - DeviceResp *oauth2.DeviceAuthResponse - token *oauth2.Token + DeviceResp *oauth2.DeviceAuthResponse + token *oauth2.Token + loginCalled bool } func (ha *HttpAuth) DeviceAuth(ctx context.Context, opts ...oauth2.AuthCodeOption) (*oauth2.DeviceAuthResponse, error) { + ha.loginCalled = true return ha.DeviceResp, nil } @@ -70,7 +145,7 @@ func (ds *DummyStore) storeAccessToken(token *oauth2.Token) error { } func (ds *DummyStore) readAccessToken() (*oauth2.Token, error) { ds.tokenRead = true - if ds.token != nil { + if ds.token == nil { return nil, fmt.Errorf("no token found") } return ds.token, nil