Skip to content

Commit 55c660e

Browse files
CyberArk(identity): remove backoff library and simplify error handling
- Eliminated the use of the backoff library for retry logic in authentication methods - Simplified error handling by directly returning errors without wrapping them in backoff.Permanent. - Removed unnecessary retry logic for 4xx errors in `doStartAuthentication` and `doAdvanceAuthentication`. - Improved code readability by streamlining the `LoginUsernamePassword` function. Signed-off-by: Richard Wall <richard.wall@cyberark.com>
1 parent f4c60f2 commit 55c660e

File tree

1 file changed

+15
-32
lines changed

1 file changed

+15
-32
lines changed

pkg/internal/cyberark/identity/identity.go

Lines changed: 15 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,7 @@ import (
99
"net/http"
1010
"net/url"
1111
"sync"
12-
"time"
1312

14-
"github.com/cenkalti/backoff/v5"
1513
"k8s.io/klog/v2"
1614

1715
"github.com/jetstack/preflight/pkg/logs"
@@ -209,25 +207,17 @@ func (c *Client) LoginUsernamePassword(ctx context.Context, username string, pas
209207
}
210208
}()
211209

212-
operation := func() (any, error) {
213-
advanceRequestBody, err := c.doStartAuthentication(ctx, username)
214-
if err != nil {
215-
return struct{}{}, err
216-
}
217-
218-
// NB: We explicitly pass advanceRequestBody by value here so that when we add the password
219-
// in doAdvanceAuthentication we don't create a copy of the password slice elsewhere.
220-
err = c.doAdvanceAuthentication(ctx, username, &password, advanceRequestBody)
221-
if err != nil {
222-
return struct{}{}, err
223-
}
224-
225-
return struct{}{}, nil
210+
advanceRequestBody, err := c.doStartAuthentication(ctx, username)
211+
if err != nil {
212+
return err
226213
}
227214

228-
backoffPolicy := backoff.NewConstantBackOff(10 * time.Second)
229-
230-
_, err := backoff.Retry(ctx, operation, backoff.WithBackOff(backoffPolicy))
215+
// NB: We explicitly pass advanceRequestBody by value here so that when we add the password
216+
// in doAdvanceAuthentication we don't create a copy of the password slice elsewhere.
217+
err = c.doAdvanceAuthentication(ctx, username, &password, advanceRequestBody)
218+
if err != nil {
219+
return err
220+
}
231221

232222
return err
233223
}
@@ -281,8 +271,7 @@ func (c *Client) doStartAuthentication(ctx context.Context, username string) (ad
281271
}
282272

283273
// If we got a 4xx error, we shouldn't retry
284-
return response, backoff.Permanent(err)
285-
274+
return response, err
286275
}
287276

288277
startAuthResponse := startAuthenticationResponseBody{}
@@ -354,19 +343,19 @@ func (c *Client) doStartAuthentication(ctx context.Context, username string) (ad
354343
// and receiving a token in response.
355344
func (c *Client) doAdvanceAuthentication(ctx context.Context, username string, password *[]byte, requestBody advanceAuthenticationRequestBody) error {
356345
if password == nil {
357-
return backoff.Permanent(fmt.Errorf("password must not be nil; this is a programming error"))
346+
return fmt.Errorf("password must not be nil; this is a programming error")
358347
}
359348

360349
requestBody.Answer = string(*password)
361350

362351
bodyJSON, err := json.Marshal(requestBody)
363352
if err != nil {
364-
return backoff.Permanent(fmt.Errorf("failed to marshal JSON for request to AdvanceAuthentication endpoint: %s", err))
353+
return fmt.Errorf("failed to marshal JSON for request to AdvanceAuthentication endpoint: %s", err)
365354
}
366355

367356
endpoint, err := url.JoinPath(c.baseURL, "Security", "AdvanceAuthentication")
368357
if err != nil {
369-
return backoff.Permanent(fmt.Errorf("failed to create URL for request to CyberArk Identity AdvanceAuthentication: %s", err))
358+
return fmt.Errorf("failed to create URL for request to CyberArk Identity AdvanceAuthentication: %s", err)
370359
}
371360

372361
request, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, bytes.NewReader(bodyJSON))
@@ -386,13 +375,7 @@ func (c *Client) doAdvanceAuthentication(ctx context.Context, username string, p
386375
// Important: Even login failures can produce a 200 status code, so this
387376
// check won't catch all failures
388377
if httpResponse.StatusCode != http.StatusOK {
389-
err := fmt.Errorf("got unexpected status code %s from request to advance authentication in CyberArk Identity API", httpResponse.Status)
390-
if httpResponse.StatusCode >= 500 || httpResponse.StatusCode < 400 {
391-
return err
392-
}
393-
394-
// If we got a 4xx error, we shouldn't retry
395-
return backoff.Permanent(err)
378+
return fmt.Errorf("got unexpected status code %s from request to advance authentication in CyberArk Identity API", httpResponse.Status)
396379
}
397380

398381
advanceAuthResponse := advanceAuthenticationResponseBody{}
@@ -413,7 +396,7 @@ func (c *Client) doAdvanceAuthentication(ctx context.Context, username string, p
413396
if advanceAuthResponse.Result.Summary != SummaryLoginSuccess {
414397
// IF MFA was enabled and we got here, there's probably nothing to be gained from a retry
415398
// and the best thing to do is fail now so the user can fix MFA settings.
416-
return backoff.Permanent(fmt.Errorf("got a %s response from AdvanceAuthentication; this implies that the user account %s requires MFA, which is not supported. Try unlocking MFA for this user", advanceAuthResponse.Result.Summary, username))
399+
return fmt.Errorf("got a %s response from AdvanceAuthentication; this implies that the user account %s requires MFA, which is not supported. Try unlocking MFA for this user", advanceAuthResponse.Result.Summary, username)
417400
}
418401

419402
klog.FromContext(ctx).Info("successfully completed AdvanceAuthentication request to CyberArk Identity; login complete", "username", username)

0 commit comments

Comments
 (0)