Skip to content

fix(gotrue): classify auth errors by error_code #431

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

Draft
Tr00d wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

fix(gotrue): classify auth errors by error_code #431

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 5 additions & 2 deletions packages/Gotrue/Gotrue.Tests/Authentication/AuthenticationFailureTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,13 @@ public async Task SignUp_ShouldThrowUserBadPassword_GivenWeakPassword()
}

[TestMethod]
public async Task SignUp_ShouldThrowUserBadEmailAddress_GivenInvalidEmail()
public async Task SignUp_ShouldSurfaceTheServerErrorCode_GivenInvalidEmail()
{
var signUp = () => this.Client.SignUp("not a real email address", Password);
await this.VerifyRejected(signUp, UserBadEmailAddress);
var exception = await signUp.Should().ThrowAsync<GotrueException>();
exception.Which.ErrorCode.Should().Be("validation_failed",
"GoTrue rejects a malformed email with the generic validation_failed code, which the SDK surfaces verbatim rather than guessing a finer reason from the message text");
this.StateChanges.Should().BeEmpty();
}

[TestMethod]
Expand Down
115 changes: 80 additions & 35 deletions packages/Gotrue/Gotrue.Tests/Errors/FailureHintTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

using System.Threading.Tasks;
using FluentAssertions;
using FluentAssertions.Execution;
using Gotrue.Tests.Support;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Supabase.Gotrue.Exceptions;
Expand Down Expand Up @@ -32,46 +33,90 @@ public class FailureHintTests
public void TestCleanup() => this.server.Dispose();

[TestMethod]
[DataRow(400, "Invalid login credentials", UserBadLogin, DisplayName = "400 invalid login")]
[DataRow(400, "Email not confirmed", UserEmailNotConfirmed, DisplayName = "400 email not confirmed")]
[DataRow(400, "Invalid Refresh Token", InvalidRefreshToken, DisplayName = "400 invalid refresh token")]
[DataRow(400, "refresh_token_not_found", InvalidRefreshToken, DisplayName = "400 refresh token not found")]
[DataRow(400, "Refresh token is not valid", InvalidRefreshToken, DisplayName = "400 malformed refresh token")]
[DataRow(400, "Phone number is invalid", UserBadPhoneNumber, DisplayName = "400 bad phone")]
[DataRow(400, "Email address is invalid", UserBadEmailAddress, DisplayName = "400 bad email")]
[DataRow(400, "You must provide a value", UserMissingInformation, DisplayName = "400 missing information")]
[DataRow(401, "This endpoint requires a Bearer token", AdminTokenRequired, DisplayName = "401 bearer required")]
[DataRow(403, "Invalid token", AdminTokenRequired, DisplayName = "403 invalid token")]
[DataRow(403, "invalid JWT", AdminTokenRequired, DisplayName = "403 invalid JWT")]
[DataRow(404, "No SSO provider assigned for this domain", SsoDomainNotFound, DisplayName = "404 sso domain not found")]
[DataRow(404, "No such SSO provider", SsoProviderNotFound, DisplayName = "404 sso provider not found")]
[DataRow(422, "User already registered", UserAlreadyRegistered, DisplayName = "422 already registered")]
[DataRow(422, "Phone and Email are both invalid", UserBadMultiple, DisplayName = "422 bad phone and email")]
[DataRow(422, "Invalid email and password", UserBadMultiple, DisplayName = "422 bad email and password")]
[DataRow(422, "Password is too weak", UserBadPassword, DisplayName = "422 bad password")]
[DataRow(429, "Too many requests", UserTooManyRequests, DisplayName = "429 rate limited")]
[DataRow(500, "boom", Unknown, DisplayName = "unrecognized status")]
[DataRow(502, "boom", NetworkError, DisplayName = "standard server/gateway errors")]
[DataRow(503, "boom", NetworkError, DisplayName = "standard server/gateway errors")]
[DataRow(504, "boom", NetworkError, DisplayName = "standard server/gateway errors")]
[DataRow(520, "boom", CloudflareNetworkError, DisplayName = "cloudflare errors")]
[DataRow(521, "boom", CloudflareNetworkError, DisplayName = "cloudflare errors")]
[DataRow(522, "boom", CloudflareNetworkError, DisplayName = "cloudflare errors")]
[DataRow(523, "boom", CloudflareNetworkError, DisplayName = "cloudflare errors")]
[DataRow(524, "boom", CloudflareNetworkError, DisplayName = "cloudflare errors")]
[DataRow(530, "boom", CloudflareNetworkError, DisplayName = "cloudflare errors")]
public async Task DetectReason_ShouldMapServerErrorToReason(
int statusCode,
string body,
FailureHint.Reason expected
)
[DataRow(429, UserTooManyRequests, DisplayName = "429 rate limited")]
[DataRow(500, Unknown, DisplayName = "unrecognized status")]
[DataRow(502, NetworkError, DisplayName = "502 gateway error")]
[DataRow(503, NetworkError, DisplayName = "503 gateway error")]
[DataRow(504, NetworkError, DisplayName = "504 gateway error")]
[DataRow(520, CloudflareNetworkError, DisplayName = "520 cloudflare error")]
[DataRow(521, CloudflareNetworkError, DisplayName = "521 cloudflare error")]
[DataRow(522, CloudflareNetworkError, DisplayName = "522 cloudflare error")]
[DataRow(523, CloudflareNetworkError, DisplayName = "523 cloudflare error")]
[DataRow(524, CloudflareNetworkError, DisplayName = "524 cloudflare error")]
[DataRow(530, CloudflareNetworkError, DisplayName = "530 cloudflare error")]
public async Task DetectReason_ShouldFallBackToStatusCode_GivenNoErrorCode(int statusCode, FailureHint.Reason expected)
{
this.StubSignUp(statusCode, body);
this.StubSignUp(statusCode, "an upstream gateway page");
var signUp = () => TestClients.Against(this.server).SignUp(RandomEmail(), Password);
var exception = await signUp.Should().ThrowAsync<GotrueException>();
exception
.Which.Reason.Should()
.Be(expected, $"status {statusCode} with body \"{body}\" classifies as {expected}");
.Be(expected, $"a bodyless status {statusCode} classifies as {expected}");
}

[TestMethod]
[DataRow("invalid_credentials", UserBadLogin, DisplayName = "invalid_credentials")]
[DataRow("email_not_confirmed", UserEmailNotConfirmed, DisplayName = "email_not_confirmed")]
[DataRow("email_address_invalid", UserBadEmailAddress, DisplayName = "email_address_invalid")]
[DataRow("refresh_token_not_found", InvalidRefreshToken, DisplayName = "refresh_token_not_found")]
[DataRow("refresh_token_already_used", InvalidRefreshToken, DisplayName = "refresh_token_already_used")]
[DataRow("user_already_exists", UserAlreadyRegistered, DisplayName = "user_already_exists")]
[DataRow("email_exists", UserAlreadyRegistered, DisplayName = "email_exists")]
[DataRow("phone_exists", UserAlreadyRegistered, DisplayName = "phone_exists")]
[DataRow("weak_password", UserBadPassword, DisplayName = "weak_password")]
[DataRow("over_request_rate_limit", UserTooManyRequests, DisplayName = "over_request_rate_limit")]
[DataRow("over_email_send_rate_limit", UserTooManyRequests, DisplayName = "over_email_send_rate_limit")]
[DataRow("over_sms_send_rate_limit", UserTooManyRequests, DisplayName = "over_sms_send_rate_limit")]
[DataRow("bad_jwt", AdminTokenRequired, DisplayName = "bad_jwt")]
[DataRow("no_authorization", AdminTokenRequired, DisplayName = "no_authorization")]
[DataRow("not_admin", AdminTokenRequired, DisplayName = "not_admin")]
[DataRow("sso_provider_not_found", SsoProviderNotFound, DisplayName = "sso_provider_not_found")]
[DataRow("mfa_verification_failed", MfaChallengeUnverified, DisplayName = "mfa_verification_failed")]
[DataRow("mfa_verification_rejected", MfaChallengeUnverified, DisplayName = "mfa_verification_rejected")]
[DataRow("mfa_challenge_expired", MfaChallengeUnverified, DisplayName = "mfa_challenge_expired")]
public async Task DetectReason_ShouldMapErrorCodeToReason(string errorCode, FailureHint.Reason expected)
{
this.StubSignUp(400, $$"""{"code":400,"error_code":"{{errorCode}}","msg":"a server message"}""");
var signUp = () => TestClients.Against(this.server).SignUp(RandomEmail(), Password);
var exception = await signUp.Should().ThrowAsync<GotrueException>();
exception.Which.Reason.Should().Be(expected, $"error_code \"{errorCode}\" classifies as {expected}");
exception.Which.ErrorCode.Should().Be(errorCode, "the raw server error_code is exposed for precise handling");
}

[TestMethod]
public async Task DetectReason_ShouldClassifyFromErrorCode_GivenConflictingMessageText()
{
this.StubSignUp(400, """{"code":400,"error_code":"invalid_credentials","msg":"Email not confirmed"}""");
var signUp = () => TestClients.Against(this.server).SignUp(RandomEmail(), Password);
var exception = await signUp.Should().ThrowAsync<GotrueException>();
exception.Which.Reason.Should()
.Be(UserBadLogin, "classification comes from the machine-readable error_code, not the message text");
}

[TestMethod]
public async Task DetectReason_ShouldBeUnknown_GivenAnUnmappedErrorCode()
{
this.StubSignUp(400, """{"code":400,"error_code":"validation_failed","msg":"You must provide a value"}""");
var signUp = () => TestClients.Against(this.server).SignUp(RandomEmail(), Password);
var exception = await signUp.Should().ThrowAsync<GotrueException>();
using (new AssertionScope())
{
exception.Which.Reason.Should().Be(Unknown, "a generic/unmapped code resolves to Unknown rather than guessing from message text");
exception.Which.ErrorCode.Should().Be("validation_failed", "the raw code is still surfaced so callers can branch on it");
}
}

[TestMethod]
public async Task ErrorCode_ShouldBeNull_GivenANonJsonBody()
{
this.StubSignUp(502, "an upstream gateway page");
var signUp = () => TestClients.Against(this.server).SignUp(RandomEmail(), Password);
var exception = await signUp.Should().ThrowAsync<GotrueException>();
using (new AssertionScope())
{
exception.Which.ErrorCode.Should().BeNull("a gateway/Cloudflare page carries no error_code");
exception.Which.Reason.Should().Be(NetworkError, "status-code classification still applies");
}
}

private void StubSignUp(int statusCode, string body) =>
Expand Down
8 changes: 4 additions & 4 deletions packages/Gotrue/Gotrue.Tests/TokenRefresh/RefreshContractTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,13 @@ public async Task RefreshToken_ShouldBecomeTheCurrentSession_GivenSuccess()
[TestMethod]
[DataRow("token_not_found_error.json", DisplayName = "unknown token (refresh_token_not_found)")]
[DataRow("malformed_token_error.json", DisplayName = "malformed token (validation_failed)")]
public async Task RefreshToken_ShouldThrowInvalidRefreshTokenAndDestroySession_GivenRejected(string fixture)
public async Task RefreshToken_ShouldThrowAndDestroySession_GivenTheServerRejectsTheToken(string fixture)
{
this.MockErrorResponse(400, Fixture(fixture));
var refresh = () => this.client.RefreshToken(AccessToken, RefreshTokenValue);
var exception = await refresh.Should().ThrowAsync<GotrueException>();
exception.Which.Reason.Should().Be(InvalidRefreshToken);
this.client.CurrentSession.Should().BeNull();
await refresh.Should().ThrowAsync<GotrueException>();
this.client.CurrentSession.Should().BeNull(
"a definitive (4xx) refresh rejection destroys the session regardless of the specific error_code — GoTrue returns the generic validation_failed for a malformed token, so classification cannot gate this");
}

[TestMethod]
Expand Down
11 changes: 5 additions & 6 deletions packages/Gotrue/Gotrue.Tests/TokenRefresh/RefreshTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
using Supabase.Gotrue;
using Supabase.Gotrue.Exceptions;
using static Supabase.Gotrue.Constants.AuthState;
using static Supabase.Gotrue.Exceptions.FailureHint.Reason;

#endregion

Expand All @@ -17,7 +16,7 @@ namespace Gotrue.Tests.TokenRefresh;
/// <summary>
/// End-to-end session refresh against the live stack: refreshing rotates the refresh token and yields an
/// access token the server accepts (including for an already-expired session), while a rejected refresh
/// token fails as <see cref="FailureHint.Reason.InvalidRefreshToken" /> and destroys the session.
/// token throws a <see cref="GotrueException" /> and destroys the session.
/// </summary>
[TestClass]
[TestCategory("E2E")]
Expand All @@ -43,14 +42,14 @@ public async Task RefreshSession_ShouldSucceed_GivenExpiredSession()
[TestMethod]
[DataRow("bogus-token", DisplayName = "malformed token")]
[DataRow("abcdef012345", DisplayName = "well-formed unknown token")]
public async Task RefreshSession_ShouldThrowInvalidRefreshTokenAndDestroySession_GivenRejectedToken(string rejectedToken)
public async Task RefreshSession_ShouldThrowAndDestroySession_GivenRejectedToken(string rejectedToken)
{
await this.SignUpNewUser();
this.Client.CurrentSession!.RefreshToken = rejectedToken;
var refresh = () => this.Client.RefreshSession();
var exception = await refresh.Should().ThrowAsync<GotrueException>();
exception.Which.Reason.Should().Be(InvalidRefreshToken);
this.Client.CurrentSession.Should().BeNull();
await refresh.Should().ThrowAsync<GotrueException>();
this.Client.CurrentSession.Should().BeNull(
"a definitive (4xx) refresh rejection destroys the session — a malformed token comes back as the generic validation_failed, so this cannot depend on the specific reason");
}

private async Task VerifyRotatedSession(Session original, Session? refreshed)
Expand Down
12 changes: 9 additions & 3 deletions packages/Gotrue/Gotrue/Client.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -712,7 +712,7 @@ public async Task<Session> SetSession(string accessToken, string refreshToken, b
await this.RefreshToken();
return this.CurrentSession;
}
catch (GotrueException e) when (e.Reason is InvalidRefreshToken)
catch (GotrueException e) when (IsDefinitiveRefreshRejection(e))
{
// RefreshToken destroyed the session, unless it was replaced mid-flight.
activity.SetFailure(e);
Expand Down Expand Up @@ -785,7 +785,7 @@ public async Task RefreshToken(string accessToken, string refreshToken)
this.SetCurrentSession(result);
await this.NotifyAuthStateChangeAsync(TokenRefreshed).ConfigureAwait(false);
}
catch (GotrueException ex) when (ex.Reason is InvalidRefreshToken)
catch (GotrueException ex) when (IsDefinitiveRefreshRejection(ex))
{
activity.SetFailure(ex);
await this.ClearRejectedSessionAsync(refreshToken).ConfigureAwait(false);
Expand Down Expand Up @@ -861,7 +861,7 @@ private async Task RefreshCurrentSession(string accessToken, string refreshToken
}
await this.NotifyAuthStateChangeAsync(TokenRefreshed).ConfigureAwait(false);
}
catch (GotrueException ex) when (ex.Reason is InvalidRefreshToken)
catch (GotrueException ex) when (IsDefinitiveRefreshRejection(ex))
{
activity.SetFailure(ex);
await this.ClearRejectedSessionAsync(refreshToken).ConfigureAwait(false);
Expand Down Expand Up @@ -1149,6 +1149,12 @@ private async Task UpdateSessionAsync(Session? session, CancellationToken cancel
}
}

// A refresh the server answers with a client error (4xx other than rate limiting) will never succeed
// with that token, so the session it belonged to is definitively rejected. Transient failures -
// offline, network, 5xx, rate limiting - keep the session so the next refresh can retry.
private static bool IsDefinitiveRefreshRejection(GotrueException ex) =>
ex.StatusCode is >= 400 and < 500 and not 429;

// Only signs out the session the token belonged to.
private async Task ClearRejectedSessionAsync(string refreshToken)
{
Expand Down
Loading
Loading