From 01fedaa3c6b934edd6e0d361468d5e2d2cb58020 Mon Sep 17 00:00:00 2001 From: Mike Mondragon Date: Fri, 5 Jul 2024 12:33:00 -0700 Subject: [PATCH 1/6] Clean up make clean --- Makefile | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index e84aafc..59b4c10 100644 --- a/Makefile +++ b/Makefile @@ -21,10 +21,8 @@ build: fmtcheck go build -o $(GOBIN)/okta-aws-cli cmd/okta-aws-cli/main.go clean: - go clean -cache -testcache ./... - -clean-all: - go clean -cache -testcache -modcache ./... + rm -fr dist/ + go clean -testcache fmt: tools # Format the code @$(GOFMT) -l -w . From 446e6bd54ed533b53d3cd47c4f53166b60ad38fd Mon Sep 17 00:00:00 2001 From: Mike Mondragon Date: Mon, 8 Jul 2024 14:41:08 -0700 Subject: [PATCH 2/6] Better retry for when the cached access token has been invalidated outside of okta-aws-cli's control. Closes #207 Closes #198 --- cmd/root/web/web.go | 26 ++++++++-- internal/okta/apierror.go | 83 +++++++++++++++++++++++++++++- internal/paginator/paginator.go | 84 +++++++------------------------ internal/webssoauth/webssoauth.go | 64 ++++++++++++++--------- 4 files changed, 161 insertions(+), 96 deletions(-) diff --git a/cmd/root/web/web.go b/cmd/root/web/web.go index 7df3b96..c0a8f30 100644 --- a/cmd/root/web/web.go +++ b/cmd/root/web/web.go @@ -21,6 +21,7 @@ import ( "github.com/okta/okta-aws-cli/internal/config" cliFlag "github.com/okta/okta-aws-cli/internal/flag" + "github.com/okta/okta-aws-cli/internal/okta" "github.com/okta/okta-aws-cli/internal/webssoauth" ) @@ -82,16 +83,33 @@ func NewWebCommand() *cobra.Command { if err != nil { return err } + err = cliFlag.CheckRequiredFlags(requiredFlags) if err != nil { return err } - wsa, err := webssoauth.NewWebSSOAuthentication(config) - if err != nil { - return err + for attempt := 1; attempt <= 2; attempt++ { + wsa, err := webssoauth.NewWebSSOAuthentication(config) + if err != nil { + break + } + + err = wsa.EstablishIAMCredentials() + if err == nil { + break + } + + if apiErr, ok := err.(*okta.APIError); ok { + if apiErr.ErrorType == "invalid_grant" && webssoauth.RemoveCachedAccessToken() { + webssoauth.ConsolePrint(config, "\nCached access token appears to be stale, removing token and retrying device authorization ...\n\n") + continue + } + break + } } - return wsa.EstablishIAMCredentials() + + return err }, } diff --git a/internal/okta/apierror.go b/internal/okta/apierror.go index 337e9ed..618c28a 100644 --- a/internal/okta/apierror.go +++ b/internal/okta/apierror.go @@ -16,8 +16,87 @@ package okta +import ( + "bytes" + "encoding/json" + "fmt" + "io" + "net/http" + "strings" + + "github.com/BurntSushi/toml" +) + +const ( + // APIErrorMessageBase base API error message + APIErrorMessageBase = "the API returned an unknown error" + // APIErrorMessageWithErrorDescription API error message with description + APIErrorMessageWithErrorDescription = "the API returned an error: %s" + // APIErrorMessageWithErrorSummary API error message with summary + APIErrorMessageWithErrorSummary = "the API returned an error: %s" + // HTTPHeaderWwwAuthenticate Www-Authenticate header + HTTPHeaderWwwAuthenticate = "Www-Authenticate" +) + // APIError Wrapper for Okta API error type APIError struct { - Error string `json:"error,omitempty"` - ErrorDescription string `json:"error_description,omitempty"` + ErrorType string `json:"error"` + ErrorDescription string `json:"error_description"` + ErrorCode string `json:"errorCode,omitempty"` + ErrorSummary string `json:"errorSummary,omitempty" toml:"error_description"` + ErrorLink string `json:"errorLink,omitempty"` + ErrorID string `json:"errorId,omitempty"` + ErrorCauses []map[string]interface{} `json:"errorCauses,omitempty"` +} + +// Error String-ify the Error +func (e *APIError) Error() string { + formattedErr := APIErrorMessageBase + if e.ErrorDescription != "" { + formattedErr = fmt.Sprintf(APIErrorMessageWithErrorDescription, e.ErrorDescription) + } else if e.ErrorSummary != "" { + formattedErr = fmt.Sprintf(APIErrorMessageWithErrorSummary, e.ErrorSummary) + } + if len(e.ErrorCauses) > 0 { + var causes []string + for _, cause := range e.ErrorCauses { + for key, val := range cause { + causes = append(causes, fmt.Sprintf("%s: %v", key, val)) + } + } + formattedErr = fmt.Sprintf("%s. Causes: %s", formattedErr, strings.Join(causes, ", ")) + } + return formattedErr +} + +// NewAPIError Constructor for Okta API error, will return nil if the response +// is not an error. +func NewAPIError(resp *http.Response) error { + statusCode := resp.StatusCode + if statusCode >= http.StatusOK && statusCode < http.StatusBadRequest { + return nil + } + e := APIError{} + if (statusCode == http.StatusUnauthorized || statusCode == http.StatusForbidden) && + strings.Contains(resp.Header.Get(HTTPHeaderWwwAuthenticate), "Bearer") { + for _, v := range strings.Split(resp.Header.Get(HTTPHeaderWwwAuthenticate), ", ") { + if strings.Contains(v, "error_description") { + _, err := toml.Decode(v, &e) + if err != nil { + e.ErrorSummary = "unauthorized" + } + return &e + } + } + } + bodyBytes, _ := io.ReadAll(resp.Body) + copyBodyBytes := make([]byte, len(bodyBytes)) + copy(copyBodyBytes, bodyBytes) + _ = resp.Body.Close() + resp.Body = io.NopCloser(bytes.NewBuffer(bodyBytes)) + _ = json.NewDecoder(bytes.NewReader(copyBodyBytes)).Decode(&e) + if statusCode == http.StatusInternalServerError { + e.ErrorSummary += fmt.Sprintf(", x-okta-request-id=%s", resp.Header.Get("x-okta-request-id")) + } + return &e } diff --git a/internal/paginator/paginator.go b/internal/paginator/paginator.go index 3b156fc..57008f9 100644 --- a/internal/paginator/paginator.go +++ b/internal/paginator/paginator.go @@ -1,3 +1,19 @@ +/* + * Copyright (c) 2024-Present, Okta, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + package paginator import ( @@ -5,20 +21,17 @@ import ( "encoding/json" "encoding/xml" "errors" - "fmt" "io" "net/http" "net/url" "strings" - "github.com/BurntSushi/toml" + "github.com/okta/okta-aws-cli/internal/okta" ) const ( // HTTPHeaderWwwAuthenticate Www-Authenticate header HTTPHeaderWwwAuthenticate = "Www-Authenticate" - // APIErrorMessageBase base API error message - APIErrorMessageBase = "the API returned an unknown error" // APIErrorMessageWithErrorDescription API error message with description APIErrorMessageWithErrorDescription = "the API returned an error: %s" // APIErrorMessageWithErrorSummary API error message with summary @@ -136,7 +149,7 @@ func newPaginateResponse(r *http.Response, pgntr *Paginator) *PaginateResponse { func buildPaginateResponse(resp *http.Response, pgntr *Paginator, v interface{}) (*PaginateResponse, error) { ct := resp.Header.Get("Content-Type") response := newPaginateResponse(resp, pgntr) - err := checkResponseForError(resp) + err := okta.NewAPIError(resp) if err != nil { return response, err } @@ -167,64 +180,3 @@ func buildPaginateResponse(resp *http.Response, pgntr *Paginator, v interface{}) } return response, nil } - -func checkResponseForError(resp *http.Response) error { - statusCode := resp.StatusCode - if statusCode >= http.StatusOK && statusCode < http.StatusBadRequest { - return nil - } - e := Error{} - if (statusCode == http.StatusUnauthorized || statusCode == http.StatusForbidden) && - strings.Contains(resp.Header.Get(HTTPHeaderWwwAuthenticate), "Bearer") { - for _, v := range strings.Split(resp.Header.Get(HTTPHeaderWwwAuthenticate), ", ") { - if strings.Contains(v, "error_description") { - _, err := toml.Decode(v, &e) - if err != nil { - e.ErrorSummary = "unauthorized" - } - return &e - } - } - } - bodyBytes, _ := io.ReadAll(resp.Body) - copyBodyBytes := make([]byte, len(bodyBytes)) - copy(copyBodyBytes, bodyBytes) - _ = resp.Body.Close() - resp.Body = io.NopCloser(bytes.NewBuffer(bodyBytes)) - _ = json.NewDecoder(bytes.NewReader(copyBodyBytes)).Decode(&e) - if statusCode == http.StatusInternalServerError { - e.ErrorSummary += fmt.Sprintf(", x-okta-request-id=%s", resp.Header.Get("x-okta-request-id")) - } - return &e -} - -// Error A struct for marshalling Okta's API error response bodies -type Error struct { - ErrorMessage string `json:"error"` - ErrorDescription string `json:"error_description"` - ErrorCode string `json:"errorCode,omitempty"` - ErrorSummary string `json:"errorSummary,omitempty" toml:"error_description"` - ErrorLink string `json:"errorLink,omitempty"` - ErrorID string `json:"errorId,omitempty"` - ErrorCauses []map[string]interface{} `json:"errorCauses,omitempty"` -} - -// Error String-ify the Error -func (e *Error) Error() string { - formattedErr := APIErrorMessageBase - if e.ErrorDescription != "" { - formattedErr = fmt.Sprintf(APIErrorMessageWithErrorDescription, e.ErrorDescription) - } else if e.ErrorSummary != "" { - formattedErr = fmt.Sprintf(APIErrorMessageWithErrorSummary, e.ErrorSummary) - } - if len(e.ErrorCauses) > 0 { - var causes []string - for _, cause := range e.ErrorCauses { - for key, val := range cause { - causes = append(causes, fmt.Sprintf("%s: %v", key, val)) - } - } - formattedErr = fmt.Sprintf("%s. Causes: %s", formattedErr, strings.Join(causes, ", ")) - } - return formattedErr -} diff --git a/internal/webssoauth/webssoauth.go b/internal/webssoauth/webssoauth.go index 0d55723..5698c9a 100644 --- a/internal/webssoauth/webssoauth.go +++ b/internal/webssoauth/webssoauth.go @@ -28,7 +28,6 @@ import ( "net/url" "os" osexec "os/exec" - "os/user" "path/filepath" "regexp" "strings" @@ -723,16 +722,9 @@ func (w *WebSSOAuthentication) fetchSSOWebToken(clientID, awsFedAppID string, at return nil, err } - if resp.StatusCode != http.StatusOK { - baseErrStr := "fetching SSO web token received API response %q" - - var apiErr okta.APIError - err = json.NewDecoder(resp.Body).Decode(&apiErr) - if err != nil { - return nil, fmt.Errorf(baseErrStr, resp.Status) - } - - return nil, fmt.Errorf(baseErrStr+okta.AccessTokenErrorFormat, resp.Status, apiErr.Error, apiErr.ErrorDescription) + err = okta.NewAPIError(resp) + if err != nil { + return nil, err } token = &okta.AccessToken{} @@ -956,8 +948,8 @@ func (w *WebSSOAuthentication) accessToken(deviceAuth *okta.DeviceAuthorization) if err != nil { return backoff.Permanent(fmt.Errorf("fetching access token polling received unexpected API error body %q", string(bodyBytes))) } - if apiErr.Error != "authorization_pending" { - return backoff.Permanent(fmt.Errorf("fetching access token polling received unexpected API polling error %q - %q", apiErr.Error, apiErr.ErrorDescription)) + if apiErr.ErrorType != "authorization_pending" { + return backoff.Permanent(fmt.Errorf("fetching access token polling received unexpected API polling error %q - %q", apiErr.ErrorType, apiErr.ErrorDescription)) } return errors.New("continue polling") @@ -1120,15 +1112,37 @@ func (w *WebSSOAuthentication) isClassicOrg() bool { return false } +// cachedAccessTokenPath Path to the cached access token in $HOME/.okta/awscli-access-token.json +func cachedAccessTokenPath() (string, error) { + homeDir, err := os.UserHomeDir() + if err != nil { + return "", err + } + return filepath.Join(homeDir, dotOktaDir, tokenFileName), nil +} + +// RemoveCachedAccessToken Remove cached access token if it exists. Returns true +// if the file exists was reremoved, swallows errors otherwise. +func RemoveCachedAccessToken() bool { + accessTokenPath, err := cachedAccessTokenPath() + if err != nil { + return false + } + if os.Remove(accessTokenPath) != nil { + return false + } + + return true +} + // cachedAccessToken will returned the cached access token if it exists and is // not expired. func (w *WebSSOAuthentication) cachedAccessToken() (at *okta.AccessToken) { - homeDir, err := os.UserHomeDir() + accessTokenPath, err := cachedAccessTokenPath() if err != nil { return } - configPath := filepath.Join(homeDir, dotOktaDir, tokenFileName) - atJSON, err := os.ReadFile(configPath) + atJSON, err := os.ReadFile(accessTokenPath) if err != nil { return } @@ -1158,15 +1172,12 @@ func (w *WebSSOAuthentication) cacheAccessToken(at *okta.AccessToken) { return } - cUser, err := user.Current() + homeDir, err := os.UserHomeDir() if err != nil { return } - if cUser.HomeDir == "" { - return - } - oktaDir := filepath.Join(cUser.HomeDir, dotOktaDir) + oktaDir := filepath.Join(homeDir, dotOktaDir) // noop if dir exists err = os.MkdirAll(oktaDir, 0o700) if err != nil { @@ -1178,18 +1189,23 @@ func (w *WebSSOAuthentication) cacheAccessToken(at *okta.AccessToken) { return } - configPath := filepath.Join(cUser.HomeDir, dotOktaDir, tokenFileName) + configPath := filepath.Join(homeDir, dotOktaDir, tokenFileName) _ = os.WriteFile(configPath, atJSON, 0o600) } -func (w *WebSSOAuthentication) consolePrint(format string, a ...any) { - if w.config.IsProcessCredentialsFormat() { +// ConsolePrint printf formatted warning messages. +func ConsolePrint(config *config.Config, format string, a ...any) { + if config.IsProcessCredentialsFormat() { return } fmt.Fprintf(os.Stderr, format, a...) } +func (w *WebSSOAuthentication) consolePrint(format string, a ...any) { + ConsolePrint(w.config, format, a...) +} + // fetchAllAWSCredentialsWithSAMLRole Gets all AWS Credentials with an STS Assume Role with SAML AWS API call. func (w *WebSSOAuthentication) fetchAllAWSCredentialsWithSAMLRole(idpRolesMap map[string][]string, assertion, region string) <-chan *oaws.CredentialContainer { ccch := make(chan *oaws.CredentialContainer) From d0e59bed949851904ca1258cba8f7f6aa1d3db91 Mon Sep 17 00:00:00 2001 From: Mike Mondragon Date: Mon, 8 Jul 2024 14:59:00 -0700 Subject: [PATCH 3/6] Correct "default" profile flaw introduced in 3186268efcf4e5fd258e582ef0de714cc8ee4cbb Closes #216 --- cmd/root/root.go | 2 +- internal/config/config.go | 19 ++++++++++--------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/cmd/root/root.go b/cmd/root/root.go index 3936d57..5394197 100644 --- a/cmd/root/root.go +++ b/cmd/root/root.go @@ -76,7 +76,7 @@ func init() { { Name: config.ProfileFlag, Short: "p", - Value: "default", + Value: "", Usage: "AWS Profile", EnvVar: config.ProfileEnvVar, }, diff --git a/internal/config/config.go b/internal/config/config.go index 2b6a447..adb557c 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -420,7 +420,17 @@ func readConfig() (Attributes, error) { } } + // config loading order + // 1) command line flags 2) environment variables, 3) .env file awsProfile := viper.GetString(ProfileFlag) + // mimic AWS CLI behavior, if profile value is not set by flag check + // the ENV VAR, else set to "default" + if awsProfile == "" { + awsProfile = viper.GetString(downCase(ProfileEnvVar)) + } + if awsProfile == "" { + awsProfile = "default" + } attrs := Attributes{ AllProfiles: viper.GetBool(getFlagNameFromProfile(awsProfile, AllProfilesFlag)), @@ -454,15 +464,6 @@ func readConfig() (Attributes, error) { attrs.Format = EnvVarFormat } - // mimic AWS CLI behavior, if profile value is not set by flag check - // the ENV VAR, else set to "default" - if attrs.Profile == "" { - attrs.Profile = viper.GetString(downCase(ProfileEnvVar)) - } - if attrs.Profile == "" { - attrs.Profile = "default" - } - // Viper binds ENV VARs to a lower snake version, set the configs with them // if they haven't already been set by cli flag binding. if attrs.OrgDomain == "" { From 6e26e4a688ac0d941609fd674e4d022ff699f4ee Mon Sep 17 00:00:00 2001 From: Mike Mondragon Date: Mon, 8 Jul 2024 15:30:26 -0700 Subject: [PATCH 4/6] Print a warning at first run if otka.yaml is malformed Closes #208 --- cmd/root/web/web.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/cmd/root/web/web.go b/cmd/root/web/web.go index c0a8f30..841cdea 100644 --- a/cmd/root/web/web.go +++ b/cmd/root/web/web.go @@ -79,18 +79,24 @@ func NewWebCommand() *cobra.Command { Use: "web", Short: "Human oriented authentication and device authorization", RunE: func(cmd *cobra.Command, args []string) error { - config, err := config.EvaluateSettings() + cfg, err := config.EvaluateSettings() if err != nil { return err } + // Warn if there is an issue with okta.yaml + _, err = config.OktaConfig() + if err != nil { + webssoauth.ConsolePrint(cfg, "WARNING: issue with %s file. Run `okta-aws-cli debug` command for additional diagnosis.\nError: %+v\n", config.OktaYaml, err) + } + err = cliFlag.CheckRequiredFlags(requiredFlags) if err != nil { return err } for attempt := 1; attempt <= 2; attempt++ { - wsa, err := webssoauth.NewWebSSOAuthentication(config) + wsa, err := webssoauth.NewWebSSOAuthentication(cfg) if err != nil { break } @@ -102,7 +108,7 @@ func NewWebCommand() *cobra.Command { if apiErr, ok := err.(*okta.APIError); ok { if apiErr.ErrorType == "invalid_grant" && webssoauth.RemoveCachedAccessToken() { - webssoauth.ConsolePrint(config, "\nCached access token appears to be stale, removing token and retrying device authorization ...\n\n") + webssoauth.ConsolePrint(cfg, "\nCached access token appears to be stale, removing token and retrying device authorization ...\n\n") continue } break From b567748822b3a300b6241a63a5a4055180123a47 Mon Sep 17 00:00:00 2001 From: Mike Mondragon Date: Mon, 8 Jul 2024 15:43:25 -0700 Subject: [PATCH 5/6] Continue polling on a 400 "slow_down" Closes #199 --- internal/webssoauth/webssoauth.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/webssoauth/webssoauth.go b/internal/webssoauth/webssoauth.go index 5698c9a..0ccd0e0 100644 --- a/internal/webssoauth/webssoauth.go +++ b/internal/webssoauth/webssoauth.go @@ -948,7 +948,7 @@ func (w *WebSSOAuthentication) accessToken(deviceAuth *okta.DeviceAuthorization) if err != nil { return backoff.Permanent(fmt.Errorf("fetching access token polling received unexpected API error body %q", string(bodyBytes))) } - if apiErr.ErrorType != "authorization_pending" { + if apiErr.ErrorType != "authorization_pending" && apiErr.ErrorType != "slow_down" { return backoff.Permanent(fmt.Errorf("fetching access token polling received unexpected API polling error %q - %q", apiErr.ErrorType, apiErr.ErrorDescription)) } From 6ed20d5fbabdb18ebad0b9453a8231b69277b9c5 Mon Sep 17 00:00:00 2001 From: Mike Mondragon Date: Mon, 8 Jul 2024 16:05:11 -0700 Subject: [PATCH 6/6] Setup 2.3.0 release --- CHANGELOG.md | 14 ++++++++++++++ internal/config/config.go | 2 +- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d8e4661..02bd3ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,19 @@ # Changelog +## 2.3.0 (July 12, 2024) + +### ENHANCEMENTS + +* New command `okta-aws-cli list-profiles` helper to inspect profiles in okta.yaml [#222](https://github.com/okta/okta-aws-cli/pull/222), thanks [@pmgalea](https://github.com/pmgalea)! +* GH releases publish Windows artifact to Chocolatey [#215](https://github.com/okta/okta-aws-cli/pull/215), thanks [@monde](https://github.com/monde)! +* Better retry for when the cached access token has been invalidated outside of okta-aws-cli's control. [#220](https://github.com/okta/okta-aws-cli/pull/220), thanks [@monde](https://github.com/monde)! +* Print a warning at first run if otka.yaml is malformed. [#220](https://github.com/okta/okta-aws-cli/pull/220), thanks [@monde](https://github.com/monde)! + +### BUG FIXES + +* Correct "default" profile flaw introduced in 2.2.0 release [#220](https://github.com/okta/okta-aws-cli/pull/220), thanks [@monde](https://github.com/monde)! +* Continue polling instead of exit on a 400 "slow_down" API error [#220](https://github.com/okta/okta-aws-cli/pull/220), thanks [@monde](https://github.com/monde)! + ## 2.2.0 (July 3, 2024) ### ENHANCEMENTS diff --git a/internal/config/config.go b/internal/config/config.go index adb557c..ce2f525 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -40,7 +40,7 @@ func init() { const ( // Version app version - Version = "2.2.0" + Version = "2.3.0" //////////////////////////////////////////////////////////// // FORMATS