From 45b1d622dfa59f5dcae2bcb142efe4f79e047a37 Mon Sep 17 00:00:00 2001 From: Nick Mills-Barrett Date: Wed, 12 Aug 2026 17:02:00 +0100 Subject: [PATCH] Map LINE login errors and stop leaking raw errors into login copy Wait() returned plain errors for verification and login failures, so the provisioning API replaced them with a generic 500 M_UNKNOWN "Internal error in login step". Map them onto declared RespErrors instead, reusing the existing parseLoginErrorDetails so LINE's own short reason strings are quoted where it gives one. Also stop formatting the raw Go error into the login form instructions when no reason could be parsed. That put internal detail directly in user-facing copy; it now shows a fixed message. CreateLogin ignored flowID entirely and returned the email login for any value, including a typo. Validate it against the advertised flow ID and return bridgev2.ErrInvalidLoginFlowID otherwise, and give that ID a named constant so the flow list and the check cannot drift apart. Note this does not change the larger contract question: a rejected password is still reported as a fresh user_input step on HTTP 200 rather than an error, so clients cannot distinguish it from a legitimate next step. That is worth deciding deliberately before changing. --- pkg/connector/client_lifecycle_test.go | 4 +- pkg/connector/connector.go | 19 +++++--- pkg/connector/loginerrors.go | 67 ++++++++++++++++++++++++++ 3 files changed, 81 insertions(+), 9 deletions(-) create mode 100644 pkg/connector/loginerrors.go diff --git a/pkg/connector/client_lifecycle_test.go b/pkg/connector/client_lifecycle_test.go index e4d1a86..efd46f1 100644 --- a/pkg/connector/client_lifecycle_test.go +++ b/pkg/connector/client_lifecycle_test.go @@ -78,11 +78,11 @@ func TestLineClientDisconnectBeforeConnectRejectsStartup(t *testing.T) { func TestCreateLoginSharesFinalizationLock(t *testing.T) { connector := &LineConnector{} - firstProcess, err := connector.CreateLogin(context.Background(), nil, "") + firstProcess, err := connector.CreateLogin(context.Background(), nil, LoginFlowIDEmail) if err != nil { t.Fatalf("first CreateLogin returned error: %v", err) } - secondProcess, err := connector.CreateLogin(context.Background(), nil, "") + secondProcess, err := connector.CreateLogin(context.Background(), nil, LoginFlowIDEmail) if err != nil { t.Fatalf("second CreateLogin returned error: %v", err) } diff --git a/pkg/connector/connector.go b/pkg/connector/connector.go index cae3a39..f00544c 100644 --- a/pkg/connector/connector.go +++ b/pkg/connector/connector.go @@ -145,15 +145,20 @@ func (lc *LineConnector) LoadUserLogin(ctx context.Context, login *bridgev2.User return nil } +const LoginFlowIDEmail = "dev.highest.matrix.line.email_login" + func (lc *LineConnector) GetLoginFlows() []bridgev2.LoginFlow { return []bridgev2.LoginFlow{{ Name: "Login", Description: "Login with your LINE Email and Password", - ID: "dev.highest.matrix.line.email_login", + ID: LoginFlowIDEmail, }} } func (lc *LineConnector) CreateLogin(ctx context.Context, user *bridgev2.User, flowID string) (bridgev2.LoginProcess, error) { + if flowID != LoginFlowIDEmail { + return nil, bridgev2.ErrInvalidLoginFlowID + } return &LineEmailLogin{User: user, finalizeMu: &lc.loginFinalizeMu}, nil } @@ -233,7 +238,7 @@ func (ll *LineEmailLogin) StartWithOverride(ctx context.Context, override *bridg ll.logLoginFailure(err, "reconnect") reason := loginErrorReason(err) if reason == "" { - reason = fmt.Sprintf("Login failed: %v", err) + reason = genericLoginFailureReason } return ll.loginErrorStep(reason), nil } @@ -260,7 +265,7 @@ func (ll *LineEmailLogin) SubmitUserInput(ctx context.Context, input map[string] ll.logLoginFailure(err, "credentials") reason := loginErrorReason(err) if reason == "" { - reason = fmt.Sprintf("Login failed: %v", err) + reason = genericLoginFailureReason } return ll.loginErrorStep(reason), nil } @@ -456,10 +461,10 @@ func (ll *LineEmailLogin) Wait(ctx context.Context) (*bridgev2.LoginStep, error) if res.AuthToken != "" { return ll.finishLogin(ctx, res) } - return nil, fmt.Errorf("verification failed: no auth token received") + return nil, ErrLoginVerificationFailed case err := <-ll.pollErr: ll.logLoginFailure(err, "verification_poll") - return nil, fmt.Errorf("verification failed: %w", err) + return nil, wrapLineLoginError(err) case <-ctx.Done(): return nil, ctx.Err() } @@ -469,7 +474,7 @@ func (ll *LineEmailLogin) Wait(ctx context.Context) (*bridgev2.LoginStep, error) res, err := loginWithCredentials(ll.Email, ll.Password, ll.Certificate) if err != nil { ll.logLoginFailure(err, "pin_continuation") - return nil, fmt.Errorf("login failed: %w", err) + return nil, wrapLineLoginError(err) } return ll.handleLoginResponse(ctx, res) } @@ -581,7 +586,7 @@ func (ll *LineEmailLogin) finishLogin(ctx context.Context, res *line.LoginResult ll.User.Bridge.Log.Info().Int("keys", len(meta.ExportedKeyMap)).Msg("Preserved existing E2EE keys after re-login") } if !res.NoE2EE && len(meta.ExportedKeyMap) == 0 { - return nil, fmt.Errorf("LINE login completed without E2EE keychain; please reconnect again and complete the LINE verification prompt") + return nil, ErrLoginNoKeychain } detectedLineID := networkid.UserLoginID(mid) diff --git a/pkg/connector/loginerrors.go b/pkg/connector/loginerrors.go new file mode 100644 index 0000000..16d6df4 --- /dev/null +++ b/pkg/connector/loginerrors.go @@ -0,0 +1,67 @@ +package connector + +import ( + "fmt" + "net/http" + + "maunium.net/go/mautrix/bridgev2" +) + +// genericLoginFailureReason is shown in the login form when LINE rejects the sign-in but +// gives no reason we can quote. It replaces formatting the raw Go error into the +// instructions, which leaked internal detail into user-facing copy. +const genericLoginFailureReason = "LINE rejected the sign-in. Please check your email and password and try again." + +var ( + ErrLoginVerificationFailed = bridgev2.RespError{ + ErrCode: "DEV.HIGHEST.LINE.VERIFICATION_FAILED", + Err: "LINE didn't confirm the verification. Please start the login again.", + StatusCode: http.StatusBadRequest, + } + ErrLoginNoKeychain = bridgev2.RespError{ + ErrCode: "DEV.HIGHEST.LINE.NO_KEYCHAIN", + Err: "LINE finished signing in without sending the encryption keychain. Please reconnect and complete the verification prompt in the LINE app.", + StatusCode: http.StatusBadRequest, + } + ErrLoginTooManyAttempts = bridgev2.RespError{ + ErrCode: "DEV.HIGHEST.LINE.TOO_MANY_ATTEMPTS", + Err: loginTooManyAttemptsReason, + StatusCode: http.StatusTooManyRequests, + } + ErrLoginRejected = bridgev2.RespError{ + ErrCode: "DEV.HIGHEST.LINE.LOGIN_REJECTED", + Err: genericLoginFailureReason, + StatusCode: http.StatusUnauthorized, + } + ErrLoginUnknown = bridgev2.RespError{ + ErrCode: "M_UNKNOWN", + Err: "Internal error logging in to LINE", + StatusCode: http.StatusInternalServerError, + } +) + +// wrapLineLoginError translates a LINE error into one the client can act on, keeping the +// original in the chain with %w so logs are unaffected. +func wrapLineLoginError(err error) error { + if err == nil { + return nil + } + mapped := ErrLoginUnknown + details := parseLoginErrorDetails(err) + reason := details.ErrorReason + if reason == "" { + reason = details.ErrorMessage + } + switch { + case isBlockedUserLoginError(reason): + mapped = ErrLoginTooManyAttempts + case details.HTTPStatus == http.StatusTooManyRequests: + mapped = ErrLoginTooManyAttempts + case details.HTTPStatus == http.StatusUnauthorized, details.HTTPStatus == http.StatusForbidden: + mapped = ErrLoginRejected + case reason != "": + // LINE's own reason strings are short and user-facing, so quote them. + mapped = ErrLoginRejected.WithMessage("%s", reason) + } + return fmt.Errorf("%w: %w", mapped, err) +}