Skip to content

Commit

Permalink
fix(profile/token): 'profile token' command must check the validity o…
Browse files Browse the repository at this point in the history 
…f the stored token. (#1339)

* fix(profile/token): 'profile token' command must check the validity of the stored token.

The 'fastly profile token' command did not check the validity
(expiration) of the stored token, which meant that it would emit an
invalid token if the stored session token (and refresh token) had
expired. This PR changes the behavior so that the validity of the
token is checked as it is for all commands which actually make use of
the token.

* Apply review feedback.
  • Loading branch information
@kpfleming
kpfleming authored Nov 8, 2024
1 parent db0266b commit 270451f
Show file tree
Hide file tree
Showing 4 changed files with 139 additions and 25 deletions.
108 changes: 90 additions & 18 deletions pkg/commands/profile/profile_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@ import (
"fmt"
"path/filepath"
"testing"
"time"

"github.com/fastly/go-fastly/v9/fastly"

root "github.com/fastly/cli/pkg/commands/profile"
"github.com/fastly/cli/pkg/config"
"github.com/fastly/cli/pkg/mock"
"github.com/fastly/cli/pkg/testutil"
fsttime "github.com/fastly/cli/pkg/time"
)

func TestProfileCreate(t *testing.T) {
Expand Down Expand Up @@ -398,6 +400,8 @@ func TestProfileSwitch(t *testing.T) {
}

func TestProfileToken(t *testing.T) {
now := time.Now()

scenarios := []testutil.CLIScenario{
{
Name: "validate the active profile token is displayed by default",
Expand All @@ -417,14 +421,18 @@ func TestProfileToken(t *testing.T) {
ConfigFile: &config.File{
Profiles: config.Profiles{
"foo": &config.Profile{
Default: true,
Email: "[email protected]",
Token: "123",
Default: true,
Email: "[email protected]",
Token: "123",
RefreshTokenCreated: now.Unix(),
RefreshTokenTTL: 600,
},
"bar": &config.Profile{
Default: false,
Email: "[email protected]",
Token: "456",
Default: false,
Email: "[email protected]",
Token: "456",
RefreshTokenCreated: now.Unix(),
RefreshTokenTTL: 600,
},
},
},
Expand All @@ -449,14 +457,18 @@ func TestProfileToken(t *testing.T) {
ConfigFile: &config.File{
Profiles: config.Profiles{
"foo": &config.Profile{
Default: true,
Email: "[email protected]",
Token: "123",
Default: true,
Email: "[email protected]",
Token: "123",
RefreshTokenCreated: now.Unix(),
RefreshTokenTTL: 600,
},
"bar": &config.Profile{
Default: false,
Email: "[email protected]",
Token: "456",
Default: false,
Email: "[email protected]",
Token: "456",
RefreshTokenCreated: now.Unix(),
RefreshTokenTTL: 600,
},
},
},
Expand All @@ -481,14 +493,18 @@ func TestProfileToken(t *testing.T) {
ConfigFile: &config.File{
Profiles: config.Profiles{
"foo": &config.Profile{
Default: true,
Email: "[email protected]",
Token: "123",
Default: true,
Email: "[email protected]",
Token: "123",
RefreshTokenCreated: now.Unix(),
RefreshTokenTTL: 600,
},
"bar": &config.Profile{
Default: false,
Email: "[email protected]",
Token: "456",
Default: false,
Email: "[email protected]",
Token: "456",
RefreshTokenCreated: now.Unix(),
RefreshTokenTTL: 600,
},
},
},
Expand All @@ -512,6 +528,62 @@ func TestProfileToken(t *testing.T) {
},
WantError: "profile 'unknown' does not exist",
},
{
Name: "validate that an expired token generates an error",
Env: &testutil.EnvConfig{
Opts: &testutil.EnvOpts{
Copy: []testutil.FileIO{
{
Src: filepath.Join("testdata", "config.toml"),
Dst: "config.toml",
},
},
},
EditScenario: func(scenario *testutil.CLIScenario, rootdir string) {
scenario.ConfigPath = filepath.Join(rootdir, "config.toml")
},
},
ConfigFile: &config.File{
Profiles: config.Profiles{
"foo": &config.Profile{
Default: true,
Email: "[email protected]",
Token: "123",
RefreshTokenCreated: now.Add(time.Duration(-1200) * time.Second).Unix(),
RefreshTokenTTL: 600,
},
},
},
WantError: fmt.Sprintf("the token in profile 'foo' expired at '%s'", now.Add(time.Duration(-600)*time.Second).UTC().Format(fsttime.Format)),
},
{
Name: "validate that a soon-to-expire token generates an error",
Env: &testutil.EnvConfig{
Opts: &testutil.EnvOpts{
Copy: []testutil.FileIO{
{
Src: filepath.Join("testdata", "config.toml"),
Dst: "config.toml",
},
},
},
EditScenario: func(scenario *testutil.CLIScenario, rootdir string) {
scenario.ConfigPath = filepath.Join(rootdir, "config.toml")
},
},
ConfigFile: &config.File{
Profiles: config.Profiles{
"foo": &config.Profile{
Default: true,
Email: "[email protected]",
Token: "123",
RefreshTokenCreated: now.Unix(),
RefreshTokenTTL: 30,
},
},
},
WantError: fmt.Sprintf("the token in profile 'foo' will expire at '%s'", now.Add(time.Duration(30)*time.Second).UTC().Format(fsttime.Format)),
},
}

testutil.RunCLIScenarios(t, []string{root.CommandName, "token"}, scenarios)
Expand Down
45 changes: 38 additions & 7 deletions pkg/commands/profile/token.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,15 @@ package profile
import (
"fmt"
"io"
"time"

"github.com/fastly/cli/pkg/argparser"
"github.com/fastly/cli/pkg/config"
fsterr "github.com/fastly/cli/pkg/errors"
"github.com/fastly/cli/pkg/global"
"github.com/fastly/cli/pkg/profile"
"github.com/fastly/cli/pkg/text"
fsttime "github.com/fastly/cli/pkg/time"
)

// TokenCommand represents a Kingpin command.
Expand All @@ -26,32 +29,42 @@ func NewTokenCommand(parent argparser.Registerer, g *global.Data) *TokenCommand
return &c
}

// By default tokens must be valid for at least 5 minutes to be
// considered valid.
const defaultTokenTTL time.Duration = 5 * time.Minute

// Exec implements the command interface.
func (c *TokenCommand) Exec(_ io.Reader, out io.Writer) (err error) {
var p string
var name string
if c.profile != "" {
p = c.profile
name = c.profile
}
if c.Globals.Flags.Profile != "" {
p = c.Globals.Flags.Profile
name = c.Globals.Flags.Profile
// NOTE: If global --profile is set, it take precedence over 'profile' arg.
// It's unlikely someone will provide both, but we'll code defensively.
}

if p != "" {
if p := profile.Get(p, c.Globals.Config.Profiles); p != nil {
if name != "" {
if p := profile.Get(name, c.Globals.Config.Profiles); p != nil {
if err = checkTokenValidity(name, p, defaultTokenTTL); err != nil {
return err
}
text.Output(out, p.Token)
return nil
}
msg := fmt.Sprintf(profile.DoesNotExist, p)
msg := fmt.Sprintf(profile.DoesNotExist, name)
return fsterr.RemediationError{
Inner: fmt.Errorf(msg),
Remediation: fsterr.ProfileRemediation,
}
}

// If no 'profile' arg or global --profile, then we'll use 'active' profile.
if _, p := profile.Default(c.Globals.Config.Profiles); p != nil {
if name, p := profile.Default(c.Globals.Config.Profiles); p != nil {
if err = checkTokenValidity(name, p, defaultTokenTTL); err != nil {
return err
}
text.Output(out, p.Token)
return nil
}
Expand All @@ -60,3 +73,21 @@ func (c *TokenCommand) Exec(_ io.Reader, out io.Writer) (err error) {
Remediation: fsterr.ProfileRemediation,
}
}

func checkTokenValidity(profileName string, p *config.Profile, ttl time.Duration) (err error) {
var msg string
expiry := time.Unix(p.RefreshTokenCreated, 0).Add(time.Duration(p.RefreshTokenTTL) * time.Second)

if expiry.After(time.Now().Add(ttl)) {
return nil
} else if expiry.Before(time.Now()) {
msg = fmt.Sprintf(profile.TokenExpired, profileName, expiry.UTC().Format(fsttime.Format))
} else {
msg = fmt.Sprintf(profile.TokenWillExpire, profileName, expiry.UTC().Format(fsttime.Format))
}

return fsterr.RemediationError{
Inner: fmt.Errorf(msg),
Remediation: fsterr.TokenExpirationRemediation,
}
}
5 changes: 5 additions & 0 deletions pkg/errors/remediation_error.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,3 +165,8 @@ var InvalidStaticConfigRemediation = strings.Join([]string{
"If this does not resolve the issue, then please file an issue:",
"https://github.com/fastly/cli/issues/new?labels=bug&template=bug_report.md",
}, " ")

// TokenExpirationRemediation indicates that a stored token has expired.
var TokenExpirationRemediation = strings.Join([]string{
"Run 'fastly --profile <NAME> update' to refresh the token.",
}, " ")
6 changes: 6 additions & 0 deletions pkg/profile/profile.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,12 @@ const DoesNotExist = "the profile '%s' does not exist"
// NoDefaults describes an output warning message.
const NoDefaults = "At least one account profile should be set as the 'default'. Run `fastly profile update <NAME>` and ensure the profile is set to be the default."

// TokenExpired is a token expiration error message.
const TokenExpired = "the token in profile '%s' expired at '%s'"

// TokenWillExpire is a token expiration error message.
const TokenWillExpire = "the token in profile '%s' will expire at '%s'"

// Exist reports whether the given profile exists.
func Exist(name string, p config.Profiles) bool {
for k := range p {
Expand Down

0 comments on commit 270451f

Please sign in to comment.