pomerium / datasource

external data source contrib
Apache License 2.0
1 stars 1 forks source link

datasource/okta: rate limits not respected #341

Closed calebdoxsey closed 2 months ago

calebdoxsey commented 3 months ago

What happened?

Okta API requests sometimes return 429 errors:

{
  "level": "debug",
  "method": "GET",
  "authority": "AUTHORITY",
  "path": "/api/v1/groups/GROUPID/users",
  "duration": 104.973251,
  "response-code": 429,
  "idp": "okta",
  "response-body": "{\"errorCode\":\"E0000047\",\"errorSummary\":\"API call exceeded rate limit due to too many requests.\",\"errorLink\":\"E0000047\",\"errorId\":\"oaeXtysydrqTluVFFtCX3H8IA\",\"errorCauses\":[]}",
  "time": "2024-06-19T12:35:05Z",
  "message": "http-request"
}

What did you expect to happen?

Okta returns rate limits in API response headers: https://developer.okta.com/docs/reference/rl-best-practices

We should be able to retry the request intelligently based on these headers.

Additional context

We should also investigate if there's a way to make less calls to the API to get the same membership data.

kenjenkins commented 3 months ago

Per Caleb: it looks like we do honor the rate limit headers.

calderonth commented 3 months ago

I'm still seeing constant sync errors though.

On Mon, 24 Jun 2024, 17:56 Kenneth Jenkins, @.***> wrote:

Per Caleb: it looks like we do honor the rate limit headers.

— Reply to this email directly, view it on GitHub https://github.com/pomerium/datasource/issues/341#issuecomment-2187012142, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAF3DVXNZ3GN55O664VUJBLZJBFTJAVCNFSM6AAAAABJSGOR3OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOBXGAYTEMJUGI . You are receiving this because you are subscribed to this thread.Message ID: @.***>

calebdoxsey commented 3 months ago

In testing I do see that the rate limiting code appears to function as intended. However I uncovered another issue: #342.

calebdoxsey commented 2 months ago

I believe the sync issues were due to the code always doing a full re-sync. The Okta identity provider has been rewritten to use the Okta SDK and should only be syncing changes now.