-
Notifications
You must be signed in to change notification settings - Fork 326
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.
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
Spar re-login deleted sso users; fix handling of brig errors. #588
Conversation
a97d72d
to
87d9e2f
Compare
Ping when this can be reviewed |
87d9e2f
to
dbffd78
Compare
i'm actually not sure what to do with the intra calls to brig if they do not return 2xx. i'll think about it some more tomorrow. |
dbffd78
to
ba9a458
Compare
ping @neongreen all integration tests pass locally. |
services/spar/src/Spar/App.hs
Outdated
@@ -269,9 +269,12 @@ verdictHandlerResult bindCky = catchVerdictErrors . \case | |||
| uid == uid' -> pure uid -- redundant binding (no change to brig or spar) | |||
| otherwise -> throwSpar SparBindUserRefTaken -- attempt to use ssoid for a second wire user | |||
|
|||
cky :: SetCookie <- Intra.ssoLogin uid | |||
SAML.logger SAML.Debug (show uid) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be better to have a message here as well, not just a lonely UUID
services/spar/src/Spar/Error.hs
Outdated
@@ -93,6 +94,8 @@ sparToWaiError (SAML.CustomError SparNoBodyInBrigResponse) = Righ | |||
sparToWaiError (SAML.CustomError (SparCouldNotParseBrigResponse msg)) = Right $ Wai.Error status502 "bad-upstream" ("Could not parse response body: " <> msg) | |||
sparToWaiError (SAML.CustomError (SparBrigError msg)) = Right $ Wai.Error status500 "bad-upstream" msg | |||
-- Galley-specific errors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line should be deleted
|
||
-- second login | ||
|
||
liftIO $ threadDelay 1000000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
100000
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also a comment about "why it's sensible to leave this threadDelay here" will be useful when we both forget in the future.
No description provided.