From 87ccafbcb30377e76d435c23f701c8fe0b5006e3 Mon Sep 17 00:00:00 2001 From: Annie Ku Date: Wed, 4 Oct 2023 09:57:08 -0700 Subject: [PATCH 1/7] wip: attempt at making the oidc_impl package more general --- oidc_cli/oidc_impl/client/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/oidc_cli/oidc_impl/client/client.go b/oidc_cli/oidc_impl/client/client.go index 1d7e4688..5ee28413 100644 --- a/oidc_cli/oidc_impl/client/client.go +++ b/oidc_cli/oidc_impl/client/client.go @@ -223,7 +223,7 @@ func (c *Client) Authenticate(ctx context.Context) (*Token, error) { } c.server.Start(ctx, c, oauthMaterial) - fmt.Fprintf(os.Stderr, "Opening browser in order to authenticate with Okta, hold on a brief second...\n") + fmt.Fprintf(os.Stderr, "Opening browser in order to authenticate with your Identity Provider, hold on a brief second...\n") time.Sleep(2 * time.Second) // intercept these outputs, send them back on error From 38c50fe19974cf695d640033025934857af89e4d Mon Sep 17 00:00:00 2001 From: Annie Ku Date: Wed, 4 Oct 2023 10:01:17 -0700 Subject: [PATCH 2/7] reduce scopes as a test --- oidc_cli/oidc_impl/client/client.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/oidc_cli/oidc_impl/client/client.go b/oidc_cli/oidc_impl/client/client.go index 5ee28413..b76b467b 100644 --- a/oidc_cli/oidc_impl/client/client.go +++ b/oidc_cli/oidc_impl/client/client.go @@ -53,9 +53,7 @@ func NewClient(ctx context.Context, config *Config, clientOptions ...Option) (*C Endpoint: provider.Endpoint(), Scopes: []string{ oidc.ScopeOpenID, - oidc.ScopeOfflineAccess, "email", - "groups", }, } From 839ab131459d758b118120c409ecacba910cd2cd Mon Sep 17 00:00:00 2001 From: Annie Ku Date: Wed, 4 Oct 2023 10:14:18 -0700 Subject: [PATCH 3/7] add print statements to debug size issue --- oidc_cli/oidc_impl/cache/cache.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/oidc_cli/oidc_impl/cache/cache.go b/oidc_cli/oidc_impl/cache/cache.go index 2cd97228..bba52487 100644 --- a/oidc_cli/oidc_impl/cache/cache.go +++ b/oidc_cli/oidc_impl/cache/cache.go @@ -2,6 +2,7 @@ package cache import ( "context" + "fmt" "github.com/chanzuckerberg/go-misc/oidc_cli/oidc_impl/client" "github.com/chanzuckerberg/go-misc/oidc_cli/oidc_impl/storage" @@ -39,6 +40,9 @@ func (c *Cache) Read(ctx context.Context) (*client.Token, error) { if err != nil { return nil, err } + if cachedToken == nil { + fmt.Println("no cached Token") + } // if we have a valid token, use it if cachedToken.IsFresh() { return cachedToken, nil @@ -83,6 +87,7 @@ func (c *Cache) refresh(ctx context.Context) (*client.Token, error) { if err != nil { return nil, errors.Wrap(err, "unable to marshall token") } + fmt.Println("about to put strToken to storage: ", strToken) // save token to storage err = c.storage.Set(ctx, strToken) if err != nil { From 6363dc9ab6fa0bffef3c47a9d69c36099442859a Mon Sep 17 00:00:00 2001 From: Annie Ku Date: Wed, 4 Oct 2023 10:28:11 -0700 Subject: [PATCH 4/7] see if token stays valid after skipping caching --- oidc_cli/oidc_impl/cache/cache.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/oidc_cli/oidc_impl/cache/cache.go b/oidc_cli/oidc_impl/cache/cache.go index bba52487..4bf407f5 100644 --- a/oidc_cli/oidc_impl/cache/cache.go +++ b/oidc_cli/oidc_impl/cache/cache.go @@ -87,9 +87,9 @@ func (c *Cache) refresh(ctx context.Context) (*client.Token, error) { if err != nil { return nil, errors.Wrap(err, "unable to marshall token") } - fmt.Println("about to put strToken to storage: ", strToken) + fmt.Println("we should put this strToken to storage: ", strToken, " but putting an empty string instead") // save token to storage - err = c.storage.Set(ctx, strToken) + err = c.storage.Set(ctx, "") if err != nil { return nil, errors.Wrap(err, "Unable to cache the strToken") } From 161b65a255654587991a502e2b9876536de87304 Mon Sep 17 00:00:00 2001 From: Annie Ku Date: Wed, 4 Oct 2023 11:44:39 -0700 Subject: [PATCH 5/7] revert some print statements, try returning token without caching it --- oidc_cli/oidc_impl/cache/cache.go | 7 +----- oidc_cli/oidc_impl/storage/file.go | 2 +- oidc_cli/oidc_impl/token_getter.go | 39 +++++++++++++++--------------- 3 files changed, 22 insertions(+), 26 deletions(-) diff --git a/oidc_cli/oidc_impl/cache/cache.go b/oidc_cli/oidc_impl/cache/cache.go index 4bf407f5..2cd97228 100644 --- a/oidc_cli/oidc_impl/cache/cache.go +++ b/oidc_cli/oidc_impl/cache/cache.go @@ -2,7 +2,6 @@ package cache import ( "context" - "fmt" "github.com/chanzuckerberg/go-misc/oidc_cli/oidc_impl/client" "github.com/chanzuckerberg/go-misc/oidc_cli/oidc_impl/storage" @@ -40,9 +39,6 @@ func (c *Cache) Read(ctx context.Context) (*client.Token, error) { if err != nil { return nil, err } - if cachedToken == nil { - fmt.Println("no cached Token") - } // if we have a valid token, use it if cachedToken.IsFresh() { return cachedToken, nil @@ -87,9 +83,8 @@ func (c *Cache) refresh(ctx context.Context) (*client.Token, error) { if err != nil { return nil, errors.Wrap(err, "unable to marshall token") } - fmt.Println("we should put this strToken to storage: ", strToken, " but putting an empty string instead") // save token to storage - err = c.storage.Set(ctx, "") + err = c.storage.Set(ctx, strToken) if err != nil { return nil, errors.Wrap(err, "Unable to cache the strToken") } diff --git a/oidc_cli/oidc_impl/storage/file.go b/oidc_cli/oidc_impl/storage/file.go index 49a2112a..69f0ada3 100644 --- a/oidc_cli/oidc_impl/storage/file.go +++ b/oidc_cli/oidc_impl/storage/file.go @@ -60,7 +60,7 @@ func (f *File) Set(ctx context.Context, value string) error { return errors.Wrapf(err, "could not create cache dir %s", f.dir) } - err = ioutil.WriteFile(f.key, []byte(value), 0600) + err = os.WriteFile(f.key, []byte(value), 0600) return errors.Wrap(err, "could not set value to file") } diff --git a/oidc_cli/oidc_impl/token_getter.go b/oidc_cli/oidc_impl/token_getter.go index a6bda730..51dc7680 100644 --- a/oidc_cli/oidc_impl/token_getter.go +++ b/oidc_cli/oidc_impl/token_getter.go @@ -4,10 +4,7 @@ import ( "context" "time" - "github.com/chanzuckerberg/go-misc/oidc_cli/oidc_impl/cache" "github.com/chanzuckerberg/go-misc/oidc_cli/oidc_impl/client" - "github.com/chanzuckerberg/go-misc/oidc_cli/oidc_impl/storage" - "github.com/chanzuckerberg/go-misc/pidlock" "github.com/pkg/errors" ) @@ -18,10 +15,10 @@ const ( // GetToken gets an oidc token. // It handles caching with a default cache and keyring storage. func GetToken(ctx context.Context, clientID string, issuerURL string, clientOptions ...client.Option) (*client.Token, error) { - fileLock, err := pidlock.NewLock(lockFilePath) - if err != nil { - return nil, errors.Wrap(err, "unable to create lock") - } + // fileLock, err := pidlock.NewLock(lockFilePath) + // if err != nil { + // return nil, errors.Wrap(err, "unable to create lock") + // } conf := &client.Config{ ClientID: clientID, @@ -39,19 +36,23 @@ func GetToken(ctx context.Context, clientID string, issuerURL string, clientOpti return nil, errors.Wrap(err, "Unable to create client") } - storage, err := storage.GetOIDC(clientID, issuerURL) + token, err := c.Authenticate(ctx) if err != nil { - return nil, err - } - - cache := cache.NewCache(storage, c.RefreshToken, fileLock) - - token, err := cache.Read(ctx) - if err != nil { - return nil, errors.Wrap(err, "Unable to extract token from client") - } - if token == nil { - return nil, errors.New("nil token from OIDC-IDP") + return nil, errors.Wrap(err, "Unable to authenticate with OAuth client") } + // storage, err := storage.GetOIDC(clientID, issuerURL) + // if err != nil { + // return nil, err + // } + + // cache := cache.NewCache(storage, c.RefreshToken, fileLock) + + // token, err := cache.Read(ctx) + // if err != nil { + // return nil, errors.Wrap(err, "Unable to extract token from client") + // } + // if token == nil { + // return nil, errors.New("nil token from OIDC-IDP") + // } return token, nil } From 1c9a36408caa80b63811f07a3214938f4b029ead Mon Sep 17 00:00:00 2001 From: Annie Ku Date: Wed, 4 Oct 2023 12:24:46 -0700 Subject: [PATCH 6/7] attempt at a small feature to allow disabling the cache --- oidc_cli/oidc_impl/client/client.go | 9 ++--- oidc_cli/oidc_impl/client/config_options.go | 6 ++++ oidc_cli/oidc_impl/token_getter.go | 38 +++++++++++---------- 3 files changed, 31 insertions(+), 22 deletions(-) diff --git a/oidc_cli/oidc_impl/client/client.go b/oidc_cli/oidc_impl/client/client.go index b76b467b..41423f2d 100644 --- a/oidc_cli/oidc_impl/client/client.go +++ b/oidc_cli/oidc_impl/client/client.go @@ -18,10 +18,11 @@ import ( // Client is an oauth client type Client struct { - provider *oidc.Provider - oauthConfig *oauth2.Config - verifier *oidc.IDTokenVerifier - server *server + provider *oidc.Provider + oauthConfig *oauth2.Config + verifier *oidc.IDTokenVerifier + server *server + DisableCache bool // Extra configuration options customMessages map[oidcStatus]string diff --git a/oidc_cli/oidc_impl/client/config_options.go b/oidc_cli/oidc_impl/client/config_options.go index b0f89506..6ed31e3a 100644 --- a/oidc_cli/oidc_impl/client/config_options.go +++ b/oidc_cli/oidc_impl/client/config_options.go @@ -23,3 +23,9 @@ var SetOauth2AuthStyle = func(authStyle oauth2.AuthStyle) Option { c.oauthConfig.Endpoint.AuthStyle = authStyle } } + +var DisableTokenCache = func() Option { + return func(c *Client) { + c.DisableCache = true + } +} diff --git a/oidc_cli/oidc_impl/token_getter.go b/oidc_cli/oidc_impl/token_getter.go index 51dc7680..dc42cb87 100644 --- a/oidc_cli/oidc_impl/token_getter.go +++ b/oidc_cli/oidc_impl/token_getter.go @@ -36,23 +36,25 @@ func GetToken(ctx context.Context, clientID string, issuerURL string, clientOpti return nil, errors.Wrap(err, "Unable to create client") } - token, err := c.Authenticate(ctx) - if err != nil { - return nil, errors.Wrap(err, "Unable to authenticate with OAuth client") - } - // storage, err := storage.GetOIDC(clientID, issuerURL) - // if err != nil { - // return nil, err - // } - - // cache := cache.NewCache(storage, c.RefreshToken, fileLock) - - // token, err := cache.Read(ctx) - // if err != nil { - // return nil, errors.Wrap(err, "Unable to extract token from client") - // } - // if token == nil { - // return nil, errors.New("nil token from OIDC-IDP") - // } + if c.DisableCache: + token, err := c.Authenticate(ctx) + if err != nil { + return nil, errors.Wrap(err, "Unable to authenticate with OAuth client") + } + else: + storage, err := storage.GetOIDC(clientID, issuerURL) + if err != nil { + return nil, err + } + + cache := cache.NewCache(storage, c.RefreshToken, fileLock) + + token, err := cache.Read(ctx) + if err != nil { + return nil, errors.Wrap(err, "Unable to extract token from client") + } + if token == nil { + return nil, errors.New("nil token from OIDC-IDP") + } return token, nil } From 5541c224bbea4ceb88aeb3708360d1844ee552eb Mon Sep 17 00:00:00 2001 From: Annie Ku Date: Wed, 4 Oct 2023 12:33:20 -0700 Subject: [PATCH 7/7] fix colons--should be {} --- oidc_cli/oidc_impl/token_getter.go | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/oidc_cli/oidc_impl/token_getter.go b/oidc_cli/oidc_impl/token_getter.go index dc42cb87..8f9839ec 100644 --- a/oidc_cli/oidc_impl/token_getter.go +++ b/oidc_cli/oidc_impl/token_getter.go @@ -4,7 +4,10 @@ import ( "context" "time" + "github.com/chanzuckerberg/go-misc/oidc_cli/oidc_impl/cache" "github.com/chanzuckerberg/go-misc/oidc_cli/oidc_impl/client" + "github.com/chanzuckerberg/go-misc/oidc_cli/oidc_impl/storage" + "github.com/chanzuckerberg/go-misc/pidlock" "github.com/pkg/errors" ) @@ -15,10 +18,10 @@ const ( // GetToken gets an oidc token. // It handles caching with a default cache and keyring storage. func GetToken(ctx context.Context, clientID string, issuerURL string, clientOptions ...client.Option) (*client.Token, error) { - // fileLock, err := pidlock.NewLock(lockFilePath) - // if err != nil { - // return nil, errors.Wrap(err, "unable to create lock") - // } + fileLock, err := pidlock.NewLock(lockFilePath) + if err != nil { + return nil, errors.Wrap(err, "unable to create lock") + } conf := &client.Config{ ClientID: clientID, @@ -36,12 +39,14 @@ func GetToken(ctx context.Context, clientID string, issuerURL string, clientOpti return nil, errors.Wrap(err, "Unable to create client") } - if c.DisableCache: - token, err := c.Authenticate(ctx) + var token *client.Token + + if c.DisableCache { + token, err = c.Authenticate(ctx) if err != nil { return nil, errors.Wrap(err, "Unable to authenticate with OAuth client") } - else: + } else { storage, err := storage.GetOIDC(clientID, issuerURL) if err != nil { return nil, err @@ -56,5 +61,7 @@ func GetToken(ctx context.Context, clientID string, issuerURL string, clientOpti if token == nil { return nil, errors.New("nil token from OIDC-IDP") } + } + return token, nil }