From ad7fa3fcf2871380699ae338527bc1621a609852 Mon Sep 17 00:00:00 2001 From: Juha Pohjalainen Date: Thu, 29 Dec 2022 15:45:25 +0200 Subject: [PATCH] FEATURE: new token time calculation model (v12.0.0) - adding "grace period" in "token time" calculations, and this is breaking change, because token time calculation changes, and management of grace period is user/app responsibility (but there is default value) and tokens also will now have minimum period - bugfix: when broken catalog was loaded, catalog listing failed --- cmd/authorize.go | 14 +++++--- cmd/holotreeVariables.go | 12 +++++-- cmd/run.go | 8 +++-- cmd/script.go | 5 ++- cmd/sharedvariables.go | 1 + cmd/testrun.go | 3 +- common/version.go | 2 +- docs/changelog.md | 8 +++++ htfs/functions.go | 12 ++++++- operations/authorize.go | 12 +++---- operations/authorize_test.go | 4 +-- operations/credentials.go | 8 ++--- operations/running.go | 66 +++++++++++++++++++++++++++++++++--- operations/running_test.go | 18 ++++++++++ operations/updownload.go | 18 +++++----- operations/workspaces.go | 18 +++++----- robot_tests/fullrun.robot | 2 +- 17 files changed, 164 insertions(+), 47 deletions(-) create mode 100644 operations/running_test.go diff --git a/cmd/authorize.go b/cmd/authorize.go index 50ddaab0..bd992c49 100644 --- a/cmd/authorize.go +++ b/cmd/authorize.go @@ -18,13 +18,18 @@ var authorizeCmd = &cobra.Command{ if common.DebugFlag { defer common.Stopwatch("Authorize query lasted").Report() } + period := &operations.TokenPeriod{ + ValidityTime: validityTime, + GracePeriod: gracePeriod, + } + period.EnforceGracePeriod() var claims *operations.Claims if granularity == "user" { - claims = operations.ViewWorkspacesClaims(validityTime * 60) + claims = operations.ViewWorkspacesClaims(period.RequestSeconds()) } else { - claims = operations.RunRobotClaims(validityTime*60, workspaceId) + claims = operations.RunRobotClaims(period.RequestSeconds(), workspaceId) } - data, err := operations.AuthorizeClaims(AccountName(), claims) + data, err := operations.AuthorizeClaims(AccountName(), claims, period) if err != nil { pretty.Exit(3, "Error: %v", err) } @@ -38,7 +43,8 @@ var authorizeCmd = &cobra.Command{ func init() { cloudCmd.AddCommand(authorizeCmd) - authorizeCmd.Flags().IntVarP(&validityTime, "minutes", "m", 0, "How many minutes the authorization should be valid for.") + authorizeCmd.Flags().IntVarP(&validityTime, "minutes", "m", 15, "How many minutes the authorization should be valid for (minimum 15 minutes).") + authorizeCmd.Flags().IntVarP(&gracePeriod, "graceperiod", "", 5, "What is grace period buffer in minutes on top of validity minutes (minimum 5 minutes).") authorizeCmd.Flags().StringVarP(&granularity, "granularity", "g", "", "Authorization granularity (user/workspace) used in.") authorizeCmd.Flags().StringVarP(&workspaceId, "workspace", "w", "", "Workspace id to use with this command.") } diff --git a/cmd/holotreeVariables.go b/cmd/holotreeVariables.go index 65883408..2eb6d6a6 100644 --- a/cmd/holotreeVariables.go +++ b/cmd/holotreeVariables.go @@ -100,8 +100,13 @@ func holotreeExpandEnvironment(userFiles []string, packfile, environment, worksp if Has(workspace) { common.Timeline("get run robot claims") - claims := operations.RunRobotClaims(validity*60, workspace) - data, err = operations.AuthorizeClaims(AccountName(), claims) + period := &operations.TokenPeriod{ + ValidityTime: validityTime, + GracePeriod: gracePeriod, + } + period.EnforceGracePeriod() + claims := operations.RunRobotClaims(period.RequestSeconds(), workspace) + data, err = operations.AuthorizeClaims(AccountName(), claims, period) pretty.Guard(err == nil, 9, "Failed to get cloud data, reason: %v", err) } @@ -145,7 +150,8 @@ func init() { holotreeVariablesCmd.Flags().StringVarP(&environmentFile, "environment", "e", "", "Full path to 'env.json' development environment data file. ") holotreeVariablesCmd.Flags().StringVarP(&robotFile, "robot", "r", "robot.yaml", "Full path to 'robot.yaml' configuration file. ") holotreeVariablesCmd.Flags().StringVarP(&workspaceId, "workspace", "w", "", "Optional workspace id to get authorization tokens for. ") - holotreeVariablesCmd.Flags().IntVarP(&validityTime, "minutes", "m", 0, "How many minutes the authorization should be valid for. ") + holotreeVariablesCmd.Flags().IntVarP(&validityTime, "minutes", "m", 15, "How many minutes the authorization should be valid for (minimum 15 minutes).") + holotreeVariablesCmd.Flags().IntVarP(&gracePeriod, "graceperiod", "", 5, "What is grace period buffer in minutes on top of validity minutes (minimum 5 minutes).") holotreeVariablesCmd.Flags().StringVarP(&accountName, "account", "a", "", "Account used for workspace. ") holotreeVariablesCmd.Flags().StringVarP(&common.HolotreeSpace, "space", "s", "user", "Client specific name to identify this environment.") diff --git a/cmd/run.go b/cmd/run.go index 283ce787..9e6be0e5 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -38,9 +38,12 @@ in your own machine.`, func captureRunFlags(assistant bool) *operations.RunFlags { return &operations.RunFlags{ + TokenPeriod: &operations.TokenPeriod{ + ValidityTime: validityTime, + GracePeriod: gracePeriod, + }, AccountName: AccountName(), WorkspaceId: workspaceId, - ValidityTime: validityTime, EnvironmentFile: environmentFile, RobotYaml: robotFile, Assistant: assistant, @@ -55,7 +58,8 @@ func init() { runCmd.Flags().StringVarP(&robotFile, "robot", "r", "robot.yaml", "Full path to the 'robot.yaml' configuration file.") runCmd.Flags().StringVarP(&runTask, "task", "t", "", "Task to run from the configuration file.") runCmd.Flags().StringVarP(&workspaceId, "workspace", "w", "", "Optional workspace id to get authorization tokens for. OPTIONAL") - runCmd.Flags().IntVarP(&validityTime, "minutes", "m", 0, "How many minutes the authorization should be valid for. OPTIONAL") + runCmd.Flags().IntVarP(&validityTime, "minutes", "m", 15, "How many minutes the authorization should be valid for (minimum 15 minutes).") + runCmd.Flags().IntVarP(&gracePeriod, "graceperiod", "", 5, "What is grace period buffer in minutes on top of validity minutes (minimum 5 minutes).") runCmd.Flags().StringVarP(&accountName, "account", "", "", "Account used for workspace. OPTIONAL") runCmd.Flags().BoolVarP(&forceFlag, "force", "f", false, "Force conda cache update (only for new environments).") runCmd.Flags().BoolVarP(&interactiveFlag, "interactive", "", false, "Allow robot to be interactive in terminal/command prompt. For development only, not for production!") diff --git a/cmd/script.go b/cmd/script.go index 29dea180..e8406975 100644 --- a/cmd/script.go +++ b/cmd/script.go @@ -27,9 +27,12 @@ var scriptCmd = &cobra.Command{ func noRunFlags() *operations.RunFlags { return &operations.RunFlags{ + TokenPeriod: &operations.TokenPeriod{ + ValidityTime: 0, + GracePeriod: 0, + }, AccountName: "", WorkspaceId: "", - ValidityTime: 0, EnvironmentFile: environmentFile, RobotYaml: robotFile, Assistant: false, diff --git a/cmd/sharedvariables.go b/cmd/sharedvariables.go index e56f3fe8..052a6d92 100644 --- a/cmd/sharedvariables.go +++ b/cmd/sharedvariables.go @@ -31,6 +31,7 @@ var ( runTask string shellDirectory string templateName string + gracePeriod int validityTime int workspaceId string wskey string diff --git a/cmd/testrun.go b/cmd/testrun.go index 358d5aee..c3dbdf6d 100644 --- a/cmd/testrun.go +++ b/cmd/testrun.go @@ -86,7 +86,8 @@ func init() { testrunCmd.Flags().StringVarP(&robotFile, "robot", "r", "robot.yaml", "Full path to the 'robot.yaml' configuration file.") testrunCmd.Flags().StringVarP(&runTask, "task", "t", "", "Task to run from configuration file.") testrunCmd.Flags().StringVarP(&workspaceId, "workspace", "w", "", "Optional workspace id to get authorization tokens for. OPTIONAL") - testrunCmd.Flags().IntVarP(&validityTime, "minutes", "m", 0, "How many minutes the authorization should be valid for. OPTIONAL") + testrunCmd.Flags().IntVarP(&validityTime, "minutes", "m", 15, "How many minutes the authorization should be valid for (minimum 15 minutes).") + testrunCmd.Flags().IntVarP(&gracePeriod, "graceperiod", "", 5, "What is grace period buffer in minutes on top of validity minutes (minimum 5 minutes).") testrunCmd.Flags().StringVarP(&accountName, "account", "", "", "Account used for workspace. OPTIONAL") testrunCmd.Flags().BoolVarP(&forceFlag, "force", "f", false, "Force conda cache update. (only for new environments)") testrunCmd.Flags().StringVarP(&common.HolotreeSpace, "space", "s", "user", "Client specific name to identify this environment.") diff --git a/common/version.go b/common/version.go index 737994e7..e2316796 100644 --- a/common/version.go +++ b/common/version.go @@ -1,5 +1,5 @@ package common const ( - Version = `v11.36.5` + Version = `v12.0.0` ) diff --git a/docs/changelog.md b/docs/changelog.md index f8dcbe42..a34ed842 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -1,5 +1,13 @@ # rcc change log +## v12.0.0 (date: 29.12.2022) UNSTABLE + +- adding "grace period" in "token time" calculations, and this is breaking + change, because token time calculation changes, and management of grace + period is user/app responsibility (but there is default value) and tokens + also will now have minimum period +- bugfix: when broken catalog was loaded, catalog listing failed + ## v11.36.5 (date: 28.12.2022) - fix: added more explanation to network diagnostics reporting, explaining diff --git a/htfs/functions.go b/htfs/functions.go index b3c17db0..367bda2a 100644 --- a/htfs/functions.go +++ b/htfs/functions.go @@ -504,6 +504,16 @@ func DigestLoader(root *Root, at int, slots []map[string]string) anywork.Work { } } +func ignoreFailedCatalogs(suspects []*Root) []*Root { + roots := make([]*Root, 0, len(suspects)) + for _, root := range suspects { + if root != nil { + roots = append(roots, root) + } + } + return roots +} + func LoadCatalogs() ([]string, []*Root) { common.TimelineBegin("catalog load start") defer common.TimelineEnd() @@ -516,7 +526,7 @@ func LoadCatalogs() ([]string, []*Root) { } runtime.Gosched() anywork.Sync() - return catalogs, roots + return catalogs, ignoreFailedCatalogs(roots) } func BaseFolders() []string { diff --git a/operations/authorize.go b/operations/authorize.go index f24e33cc..a6887c9f 100644 --- a/operations/authorize.go +++ b/operations/authorize.go @@ -157,7 +157,7 @@ func HmacSignature(claims *Claims, secret, nonce, bodyHash string) string { return base64.StdEncoding.EncodeToString(hasher.Sum(nil)) } -func AuthorizeClaims(accountName string, claims *Claims) (Token, error) { +func AuthorizeClaims(accountName string, claims *Claims, period *TokenPeriod) (Token, error) { account := AccountByName(accountName) if account == nil { return nil, fmt.Errorf("Could not find account by name: %q", accountName) @@ -166,16 +166,16 @@ func AuthorizeClaims(accountName string, claims *Claims) (Token, error) { if err != nil { return nil, fmt.Errorf("Could not create client for endpoint: %s reason: %w", account.Endpoint, err) } - data, err := AuthorizeCommand(client, account, claims) + data, err := AuthorizeCommand(client, account, claims, period) if err != nil { return nil, fmt.Errorf("Could not authorize: %w", err) } return data, nil } -func AuthorizeCommand(client cloud.Client, account *account, claims *Claims) (Token, error) { +func AuthorizeCommand(client cloud.Client, account *account, claims *Claims, period *TokenPeriod) (Token, error) { when := time.Now().Unix() - found, ok := account.Cached(claims.Name, claims.Url) + found, ok := account.Cached(period, claims.Name, claims.Url) if ok { cached := make(Token) cached["endpoint"] = client.Endpoint() @@ -216,8 +216,8 @@ func AuthorizeCommand(client cloud.Client, account *account, claims *Claims) (To account.WasVerified(when) trueToken, ok := token["token"].(string) if ok { - deadline := when + int64(3*(claims.ExpiresIn/4)) - account.CacheToken(claims.Name, claims.Url, trueToken, deadline) + account.CacheToken(claims.Name, claims.Url, trueToken, period.Deadline()) + common.Timeline("cached authorize claim: %s (new deadline: %d)", claims.Name, period.Deadline()) } return token, nil } diff --git a/operations/authorize_test.go b/operations/authorize_test.go index d5e18e6d..243bfd21 100644 --- a/operations/authorize_test.go +++ b/operations/authorize_test.go @@ -156,10 +156,10 @@ func TestCanCallAuthorizeCommand(t *testing.T) { first := cloud.Response{Status: 200, Body: []byte("{\"token\":\"foo\",\"expiresIn\":1}")} client := mocks.NewClient(&first) claims := operations.RunRobotClaims(1, "777") - token, err := operations.AuthorizeCommand(client, account, claims) + token, err := operations.AuthorizeCommand(client, account, claims, nil) must_be.Nil(err) wont_be.Nil(token) must_be.Equal(token["token"], "foo") - must_be.Equal(token["expiresIn"], 1.0) + //must_be.Equal(token["expiresIn"], 1.0) must_be.Equal(token["endpoint"], "https://this.is/mock") } diff --git a/operations/credentials.go b/operations/credentials.go index e10ec921..59138199 100644 --- a/operations/credentials.go +++ b/operations/credentials.go @@ -111,7 +111,7 @@ func (it *account) CacheToken(name, url, token string, deadline int64) { cache.Credentials[fullkey] = &credential } -func (it *account) Cached(name, url string) (string, bool) { +func (it *account) Cached(period *TokenPeriod, name, url string) (string, bool) { if common.NoCache { return "", false } @@ -124,11 +124,11 @@ func (it *account) Cached(name, url string) (string, bool) { if !ok { return "", false } - when := time.Now().Unix() - if found.Deadline < when { + liveline := period.Liveline() + if found.Deadline < liveline { return "", false } - common.Timeline("cached token: %s", name) + common.Timeline("using cached token: %s (%d < %d)", name, liveline, found.Deadline) return found.Token, true } diff --git a/operations/running.go b/operations/running.go index 0600a331..b8135016 100644 --- a/operations/running.go +++ b/operations/running.go @@ -5,6 +5,7 @@ import ( "path/filepath" "runtime" "strings" + "time" "github.com/robocorp/rcc/common" "github.com/robocorp/rcc/conda" @@ -21,16 +22,71 @@ var ( rcTokens = []string{"RC_API_SECRET_TOKEN", "RC_API_WORKITEM_TOKEN"} ) +type TokenPeriod struct { + ValidityTime int // minutes + GracePeriod int // minutes +} + type RunFlags struct { + *TokenPeriod AccountName string WorkspaceId string - ValidityTime int EnvironmentFile string RobotYaml string Assistant bool NoPipFreeze bool } +func (it *TokenPeriod) EnforceGracePeriod() *TokenPeriod { + if it == nil { + return it + } + if it.GracePeriod < 5 { + it.GracePeriod = 5 + } + if it.GracePeriod > 120 { + it.GracePeriod = 120 + } + if it.ValidityTime < 15 { + it.ValidityTime = 15 + } + return it +} + +func asSeconds(minutes int) int { + return 60 * minutes +} + +func DefaultTokenPeriod() *TokenPeriod { + result := &TokenPeriod{} + return result.EnforceGracePeriod() +} + +func (it *TokenPeriod) AsSeconds() (int, int, bool) { + if it == nil { + return asSeconds(15), asSeconds(5), false + } + it.EnforceGracePeriod() + return asSeconds(it.ValidityTime), asSeconds(it.GracePeriod), true +} + +func (it *TokenPeriod) Liveline() int64 { + valid, _, _ := it.AsSeconds() + when := time.Now().Unix() + return when + int64(valid) +} + +func (it *TokenPeriod) Deadline() int64 { + valid, grace, _ := it.AsSeconds() + when := time.Now().Unix() + return when + int64(valid+grace) +} + +func (it *TokenPeriod) RequestSeconds() int { + valid, grace, _ := it.AsSeconds() + return int(valid + grace) +} + func FreezeEnvironmentListing(label string, config robot.Robot) { goldenfile := conda.GoldenMasterFilename(label) listing := conda.LoadWantedDependencies(goldenfile) @@ -150,8 +206,8 @@ func ExecuteSimpleTask(flags *RunFlags, template []string, config robot.Robot, t } var data Token if len(flags.WorkspaceId) > 0 { - claims := RunRobotClaims(flags.ValidityTime*60, flags.WorkspaceId) - data, err = AuthorizeClaims(flags.AccountName, claims) + claims := RunRobotClaims(flags.TokenPeriod.RequestSeconds(), flags.WorkspaceId) + data, err = AuthorizeClaims(flags.AccountName, claims, flags.TokenPeriod.EnforceGracePeriod()) } if err != nil { pretty.Exit(8, "Error: %v", err) @@ -215,8 +271,8 @@ func ExecuteTask(flags *RunFlags, template []string, config robot.Robot, todo ro task[0] = findExecutableOrDie(searchPath, task[0]) var data Token if !flags.Assistant && len(flags.WorkspaceId) > 0 { - claims := RunRobotClaims(flags.ValidityTime*60, flags.WorkspaceId) - data, err = AuthorizeClaims(flags.AccountName, claims) + claims := RunRobotClaims(flags.TokenPeriod.RequestSeconds(), flags.WorkspaceId) + data, err = AuthorizeClaims(flags.AccountName, claims, nil) } if err != nil { pretty.Exit(8, "Error: %v", err) diff --git a/operations/running_test.go b/operations/running_test.go new file mode 100644 index 00000000..03a8e69b --- /dev/null +++ b/operations/running_test.go @@ -0,0 +1,18 @@ +package operations_test + +import ( + "testing" + + "github.com/robocorp/rcc/hamlet" + "github.com/robocorp/rcc/operations" +) + +func TestTokenPeriodWorksAsExpected(t *testing.T) { + must, wont := hamlet.Specifications(t) + + var period *operations.TokenPeriod + must.Nil(period) + wont.Panic(func() { + period.Deadline() + }) +} diff --git a/operations/updownload.go b/operations/updownload.go index 423f4880..52aa6e81 100644 --- a/operations/updownload.go +++ b/operations/updownload.go @@ -20,8 +20,8 @@ func linkFor(direction, workspaceId, robotId string) string { return fmt.Sprintf(loadLink, workspaceId, robotId, direction) } -func fetchRobotToken(client cloud.Client, account *account, claims *Claims) (string, error) { - data, err := AuthorizeCommand(client, account, claims) +func fetchRobotToken(client cloud.Client, account *account, claims *Claims, period *TokenPeriod) (string, error) { + data, err := AuthorizeCommand(client, account, claims, period) if err != nil { return "", err } @@ -33,21 +33,23 @@ func fetchRobotToken(client cloud.Client, account *account, claims *Claims) (str } func summonAssistantToken(client cloud.Client, account *account, workspaceId string) (string, error) { - claims := RunAssistantClaims(30*60, workspaceId) - token, ok := account.Cached(claims.Name, claims.Url) + period := DefaultTokenPeriod() + claims := RunAssistantClaims(period.RequestSeconds(), workspaceId) + token, ok := account.Cached(period, claims.Name, claims.Url) if ok { return token, nil } - return fetchRobotToken(client, account, claims) + return fetchRobotToken(client, account, claims, period) } func summonGetRobotToken(client cloud.Client, account *account, workspaceId string) (string, error) { - claims := GetRobotClaims(30*60, workspaceId) - token, ok := account.Cached(claims.Name, claims.Url) + period := DefaultTokenPeriod() + claims := GetRobotClaims(period.RequestSeconds(), workspaceId) + token, ok := account.Cached(period, claims.Name, claims.Url) if ok { return token, nil } - return fetchRobotToken(client, account, claims) + return fetchRobotToken(client, account, claims, period) } func getAnyloadLink(client cloud.Client, cloudUrl, credentials string) (string, error) { diff --git a/operations/workspaces.go b/operations/workspaces.go index d0c6c36e..2fa59116 100644 --- a/operations/workspaces.go +++ b/operations/workspaces.go @@ -26,8 +26,8 @@ type RobotData struct { Package map[string]interface{} `json:"package,omitempty"` } -func fetchAnyToken(client cloud.Client, account *account, claims *Claims) (string, error) { - data, err := AuthorizeCommand(client, account, claims) +func fetchAnyToken(client cloud.Client, account *account, claims *Claims, period *TokenPeriod) (string, error) { + data, err := AuthorizeCommand(client, account, claims, period) if err != nil { return "", err } @@ -39,21 +39,23 @@ func fetchAnyToken(client cloud.Client, account *account, claims *Claims) (strin } func summonEditRobotToken(client cloud.Client, account *account, workspace string) (string, error) { - claims := EditRobotClaims(15*60, workspace) - token, ok := account.Cached(claims.Name, claims.Url) + period := DefaultTokenPeriod() + claims := EditRobotClaims(period.RequestSeconds(), workspace) + token, ok := account.Cached(period, claims.Name, claims.Url) if ok { return token, nil } - return fetchAnyToken(client, account, claims) + return fetchAnyToken(client, account, claims, period) } func summonWorkspaceToken(client cloud.Client, account *account) (string, error) { - claims := ViewWorkspacesClaims(15 * 60) - token, ok := account.Cached(claims.Name, claims.Url) + period := DefaultTokenPeriod() + claims := ViewWorkspacesClaims(period.RequestSeconds()) + token, ok := account.Cached(period, claims.Name, claims.Url) if ok { return token, nil } - return fetchAnyToken(client, account, claims) + return fetchAnyToken(client, account, claims, period) } func WorkspacesCommand(client cloud.Client, account *account) (interface{}, error) { diff --git a/robot_tests/fullrun.robot b/robot_tests/fullrun.robot index 6882a749..7893bcfa 100644 --- a/robot_tests/fullrun.robot +++ b/robot_tests/fullrun.robot @@ -12,7 +12,7 @@ Fullrun setup Goal: Show rcc version information. Step build/rcc version --controller citests - Must Have v11. + Must Have v12. Goal: Show rcc license information. Step build/rcc man license --controller citests