Skip to content

Commit

Permalink
Getting rid of complicated and buggy retry logic.
Browse files Browse the repository at this point in the history
Causes issue that surfaced in #228

Closes #228
  • Loading branch information
monde committed Aug 30, 2024
1 parent deb8118 commit 4892d65
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 53 deletions.
19 changes: 8 additions & 11 deletions cmd/root/web/web.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,10 +86,10 @@ func NewWebCommand() *cobra.Command {
return err
}

// Warn if there is an issue with okta.yaml
_, err = config.OktaConfig()
if err != nil {
if _, pathError := err.(*fs.PathError); !pathError {
// Warn if okta.yaml exists and there is an error with it.
webssoauth.ConsolePrint(cfg, "WARNING: issue with %s file. Run `okta-aws-cli debug` command for additional diagnosis.\nError: %+v\n", config.OktaYaml, err)
}
}
Expand All @@ -101,25 +101,22 @@ func NewWebCommand() *cobra.Command {

for attempt := 1; attempt <= 2; attempt++ {
wsa, err := webssoauth.NewWebSSOAuthentication(cfg)
if _, ok := err.(*webssoauth.ClassicOrgError); ok {
return err
}
if err != nil {
break
return err
}

err = wsa.EstablishIAMCredentials()
if err == nil {
break
}

if apiErr, ok := err.(*okta.APIError); ok {
if apiErr.ErrorType == "invalid_grant" && webssoauth.RemoveCachedAccessToken() {
webssoauth.ConsolePrint(cfg, "\nCached access token appears to be stale, removing token and retrying device authorization ...\n\n")
webssoauth.ConsolePrint(cfg, "Cached access token appears to be stale, removing token and retrying device authorization ...\n\n")
continue
}
break
}
if err != nil {
return err
}

break
}

return err
Expand Down
6 changes: 3 additions & 3 deletions internal/okta/apierror.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@ import (

const (
// APIErrorMessageBase base API error message
APIErrorMessageBase = "the API returned an unknown error"
APIErrorMessageBase = "Okta API returned an unknown error"
// APIErrorMessageWithErrorDescription API error message with description
APIErrorMessageWithErrorDescription = "the API returned an error: %s"
APIErrorMessageWithErrorDescription = "Okta API returned an error: %s"
// APIErrorMessageWithErrorSummary API error message with summary
APIErrorMessageWithErrorSummary = "the API returned an error: %s"
APIErrorMessageWithErrorSummary = "Okta API returned an error: %s"
// HTTPHeaderWwwAuthenticate Www-Authenticate header
HTTPHeaderWwwAuthenticate = "Www-Authenticate"
)
Expand Down
56 changes: 17 additions & 39 deletions internal/webssoauth/webssoauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,50 +149,28 @@ func (w *WebSSOAuthentication) EstablishIAMCredentials() error {
var at *okta.AccessToken
var apps []*okta.Application
var err error
at = w.cachedAccessToken()

// If there is a cached okta access token, and it isn't expired, but the API
// 401s redo the authorize step.
for attempt := 1; attempt <= 2; attempt++ {
err = nil
if at == nil {
deviceAuth, err := w.authorize()
if err != nil {
return err
}

w.promptAuthentication(deviceAuth)

at, err = w.accessToken(deviceAuth)
if err != nil {
return err
}
at.Expiry = time.Now().Add(time.Duration(at.ExpiresIn) * time.Second).Format(time.RFC3339)
w.cacheAccessToken(at)
at = w.cachedAccessToken()
if at == nil {
deviceAuth, err := w.authorize()
if err != nil {
return err
}
if w.config.FedAppID() != "" {
// Alternate path when operator knows their AWS Fed app ID
err = w.establishTokenWithFedAppID(clientID, w.config.FedAppID(), at, w.config.AWSRegion())
if at != nil && err != nil {
// possible bad cached access token, retry
at = nil
continue
}

w.promptAuthentication(deviceAuth)
at, err = w.accessToken(deviceAuth)
if err != nil {
return err
}
at.Expiry = time.Now().Add(time.Duration(at.ExpiresIn) * time.Second).Format(time.RFC3339)

w.cacheAccessToken(at)
}

if w.config.FedAppID() != "" {
return w.establishTokenWithFedAppID(clientID, w.config.FedAppID(), at, w.config.AWSRegion())
} else {

Check failure on line 172 in internal/webssoauth/webssoauth.go

View workflow job for this annotation

GitHub Actions / build

if block ends with a return statement, so drop this else and outdent its block

Check failure on line 172 in internal/webssoauth/webssoauth.go

View workflow job for this annotation

GitHub Actions / build

if block ends with a return statement, so drop this else and outdent its block
apps, err = w.listFedApps(clientID, at)
if at != nil && err != nil {
action := "exiting."
if attempt == 1 {
action = "retrying ..."
}
w.consolePrint("Listing federation apps failed, %s\n\n", action)
// possible bad cached access token, retry
at = nil
continue
}
break
}

if err != nil {
Expand Down Expand Up @@ -1075,7 +1053,7 @@ func NewClassicOrgError(orgDomain string) *ClassicOrgError {

// Error Error interface error message
func (e *ClassicOrgError) Error() string {
return fmt.Sprintf("%q is a Classic org, okta-aws-cli is an-OIE only tool", e.orgDomain)
return fmt.Sprintf("%q is a Classic org, okta-aws-cli is an OIE only tool", e.orgDomain)
}

// isClassicOrg Conduct simple check of well known endpoint to determine if the
Expand Down

0 comments on commit 4892d65

Please sign in to comment.