Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix buggy retry logic #237

Merged
merged 1 commit into from
Sep 2, 2024
Merged

Fix buggy retry logic #237

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 8 additions & 11 deletions cmd/root/web/web.go
View file
Open in desktop
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
View file
Open in desktop
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
57 changes: 17 additions & 40 deletions internal/webssoauth/webssoauth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,52 +149,29 @@ 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)
}
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
}
at = w.cachedAccessToken()
if at == nil {
deviceAuth, err := w.authorize()
if err != nil {
return err
}

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
w.promptAuthentication(deviceAuth)
at, err = w.accessToken(deviceAuth)
if err != nil {
return err
}
break
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())
}

apps, err = w.listFedApps(clientID, at)
if err != nil {
return err
}
Expand Down Expand Up @@ -1075,7 +1052,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
Loading