Skip to content

SG-786 - Fix 400 error code log outs without invalid_grant#2156

Merged
trmartin4 merged 2 commits intomasterfrom
bug/SG-786-fix-wrongly-logouts
Oct 31, 2022
Merged

SG-786 - Fix 400 error code log outs without invalid_grant#2156
trmartin4 merged 2 commits intomasterfrom
bug/SG-786-fix-wrongly-logouts

Conversation

@LRNcardozoWDF
Copy link
Member

Type of change

  • Bug fix
  • New feature development
  • Tech debt (refactoring, code cleanup, dependency upgrades, etc)
  • Build/deploy pipeline (DevOps)
  • Other

Objective

Be more precise to force log out when receiving 400 error code by ensuring that the response error is invalid grant.

Code changes

This is related to the clients PR 400s only log out on invalid grant error #3924

  • ApiService.cs: After parsing the json, log out the user if the response error is "invalid_grant".

Screenshots

Before you submit

  • Please check for formatting errors (dotnet format --verify-no-changes) (required)
  • Please add unit tests where it makes sense to do so (encouraged but not required)
  • If this change requires a documentation update - notify the documentation team
  • If this change has particular deployment requirements - notify the DevOps team

@LRNcardozoWDF LRNcardozoWDF requested a review from a team October 28, 2022 18:22
Comment on lines +799 to +811
if (authed
&&
(
(tokenError && response.StatusCode == HttpStatusCode.BadRequest && responseJObject?["error"]?.ToString() == "invalid_grant")
||
(logoutOnUnauthorized && response.StatusCode == HttpStatusCode.Unauthorized)
||
response.StatusCode == HttpStatusCode.Forbidden
))
{
await _logoutCallbackAsync(new Tuple<string, bool, bool>(null, false, true));
return null;
}
Copy link
Member

Choose a reason for hiding this comment

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

I'd split this in two:
In the same place as it was before I'd leave:

if (authed
    &&
    (
        (logoutOnUnauthorized && response.StatusCode == HttpStatusCode.Unauthorized)
        ||
        response.StatusCode == HttpStatusCode.Forbidden
    ))
{
    await _logoutCallbackAsync(new Tuple<string, bool, bool>(null, false, true));
    return null;
}

and in on the new place:

if (authed && tokenError 
    &&
    response.StatusCode == HttpStatusCode.BadRequest 
    && 
    responseJObject?["error"]?.ToString() == "invalid_grant")
{
    await _logoutCallbackAsync(new Tuple<string, bool, bool>(null, false, true));
    return null;
}

The reason behind this is that if let's say that the status code is HttpStatusCode.Unauthorized then it will waste time reading the response content; so splitting it like this we gain performance on those cases.

@trmartin4 trmartin4 merged commit ee09c0a into master Oct 31, 2022
@trmartin4 trmartin4 deleted the bug/SG-786-fix-wrongly-logouts branch October 31, 2022 17:40
trmartin4 pushed a commit that referenced this pull request Oct 31, 2022
* SG-786 - Added validation to check if the 400 error is invalid grant

* SG 786 - Improved code quality

(cherry picked from commit ee09c0a)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants