Skip to content

WPB-18562 Propagate a rate limit error with status 429 on internal call to reauthenticate #4673

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

Merged
merged 2 commits into from
Jul 23, 2025
Merged

WPB-18562 Propagate a rate limit error with status 429 on internal call to reauthenticate #4673

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ac525f3
propagate rate limit exceeded and test
battermann Jul 21, 2025
087d712
changelog
battermann Jul 21, 2025
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
1 change: 1 addition & 0 deletions changelog.d/5-internal/WPB-18562
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
A rate limit error from an internal call to `i/users/:uid/reauthenticate` will now be propagated to the external caller
12 changes: 11 additions & 1 deletion integration/test/Test/Teams.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ module Test.Teams where
import API.Brig
import qualified API.BrigInternal as I
import API.Common
import API.Galley (getTeam, getTeamMembers, getTeamMembersCsv, getTeamNotifications)
import API.Galley (deleteTeamMember, getTeam, getTeamMembers, getTeamMembersCsv, getTeamNotifications)
import qualified API.GalleyInternal as I
import API.Gundeck
import qualified API.Nginz as Nginz
Expand All @@ -30,6 +30,7 @@ import Control.Monad.Extra (findM)
import Control.Monad.Reader (asks)
import qualified Data.ByteString.Char8 as B8
import qualified Data.Map as Map
import qualified Data.Set as Set
import Data.Time.Clock
import Data.Time.Format
import Notifications
Expand Down Expand Up @@ -464,3 +465,12 @@ testUpgradeGuestToTeamShouldFail = do

upgradePersonalToTeam guest "wonderland" `bindResponse` \resp -> do
resp.status `shouldMatchInt` 404

testDeleteTeamUserRatelimitingIsPropagated :: (HasCallStack) => App ()
testDeleteTeamUserRatelimitingIsPropagated = do
(owner, tid, mems) <- createTeam OwnDomain 10
-- this is eventually going to run into rate limiting of internal request `/i/users/:uid/reauthenticate`
statusCodes <- for mems $ \m -> do
bindResponse (deleteTeamMember tid owner m) $ \resp -> do
pure resp.status
Set.fromList statusCodes `shouldMatchSet` ([202, 429] :: [Int])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we make sure that 429 was returned at least once?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This actually asserts that it's returned at least once. But I might not 100% understand your comment?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, I misread. ✅

6 changes: 6 additions & 0 deletions libs/wire-api/src/Wire/API/Error/Galley.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,25 +362,31 @@ data AuthenticationError
= ReAuthFailed
| VerificationCodeAuthFailed
| VerificationCodeRequired
| RateLimitExceeded
deriving (Show)

type instance MapError 'ReAuthFailed = 'StaticError 403 "access-denied" "This operation requires reauthentication"

type instance MapError 'VerificationCodeAuthFailed = 'StaticError 403 "code-authentication-failed" "Code authentication failed"

type instance MapError 'VerificationCodeRequired = 'StaticError 403 "code-authentication-required" "Verification code required"

type instance MapError 'RateLimitExceeded = 'StaticError 429 "too-many-requests" "Please try again later."

instance IsSwaggerError AuthenticationError where
addToOpenApi =
addStaticErrorToSwagger @(MapError 'ReAuthFailed)
. addStaticErrorToSwagger @(MapError 'VerificationCodeAuthFailed)
. addStaticErrorToSwagger @(MapError 'VerificationCodeRequired)
. addStaticErrorToSwagger @(MapError 'RateLimitExceeded)

type instance ErrorEffect AuthenticationError = Error AuthenticationError

authenticationErrorToDyn :: AuthenticationError -> DynError
authenticationErrorToDyn ReAuthFailed = dynError @(MapError 'ReAuthFailed)
authenticationErrorToDyn VerificationCodeAuthFailed = dynError @(MapError 'VerificationCodeAuthFailed)
authenticationErrorToDyn VerificationCodeRequired = dynError @(MapError 'VerificationCodeRequired)
authenticationErrorToDyn RateLimitExceeded = dynError @(MapError 'RateLimitExceeded)

instance (Member (Error DynError) r) => ServerEffect (Error AuthenticationError) r where
interpretServerEffect = mapError authenticationErrorToDyn
Expand Down
3 changes: 2 additions & 1 deletion services/galley/src/Galley/Intra/User.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,12 +148,13 @@ reAuthUser uid auth = do
method GET
. paths ["/i/users", toByteString' uid, "reauthenticate"]
. json auth
resp <- call Brig (check [status200, status403] . req)
resp <- call Brig (check [status200, status403, status429] . req)
pure $ case (statusCode . responseStatus $ resp, errorLabel resp) of
(200, _) -> Right ()
(403, Just "code-authentication-required") -> Left VerificationCodeRequired
(403, Just "code-authentication-failed") -> Left VerificationCodeAuthFailed
(403, _) -> Left ReAuthFailed
(429, _) -> Left RateLimitExceeded
(_, _) -> Left ReAuthFailed
where
errorLabel :: ResponseLBS -> Maybe Lazy.Text
Expand Down