fix(backend): report a rejected token as Unauthenticated, not as a crash

CHANGELOG.md

@@ -28,6 +28,8 @@ written while it was being built. See [RELEASING.md](RELEASING.md).
 
 ### Fixed
 
+- A server problem while joining a hackathon no longer claims that your sign-in
+  has expired. A rejected login and a real fault are told apart again.
 - Invitation links no longer fail with "This invitation is no longer valid" for
   people who have never used Hackagon before. The link was always fine — their
   account had simply never been created.

components/backend/go.mod

@@ -19,7 +19,6 @@ require (
 	github.com/MicahParks/keyfunc/v3 v3.8.0
 	github.com/casbin/casbin/v3 v3.8.1
 	github.com/casbin/ent-adapter v1.4.0
-	github.com/golang-jwt/jwt/v4 v4.4.2
 	github.com/golang-jwt/jwt/v5 v5.3.1
 	github.com/grpc-ecosystem/go-grpc-middleware v1.4.0
 	github.com/grpc-ecosystem/go-grpc-middleware/v2 v2.3.2

components/backend/go.sum

@@ -80,8 +80,6 @@ github.com/go-viper/mapstructure/v2 v2.4.0/go.mod h1:oJDH3BJKyqBA2TXFhDsKDGDTlnd
 github.com/goccy/go-yaml v1.18.0 h1:8W7wMFS12Pcas7KU+VVkaiCng+kG8QiFeFwzFb+rwuw=
 github.com/goccy/go-yaml v1.18.0/go.mod h1:XBurs7gK8ATbW4ZPGKgcbrY1Br56PdM69F7LkFRi1kA=
 github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69NZV8Q=
-github.com/golang-jwt/jwt/v4 v4.4.2 h1:rcc4lwaZgFMCZ5jxF9ABolDcIHdBytAFgqFPbSJQAYs=
-github.com/golang-jwt/jwt/v4 v4.4.2/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0=
 github.com/golang-jwt/jwt/v5 v5.3.1 h1:kYf81DTWFe7t+1VvL7eS+jKFVWaUnK9cB1qbwn63YCY=
 github.com/golang-jwt/jwt/v5 v5.3.1/go.mod h1:fxCRLWMO43lRc8nhHWY6LGqRcf+1gQWArsqaEUEa5bE=
 github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b/go.mod h1:SBH7ygxi8pfUlaOkMMuAQtPIUF8ecWP5IEl/CR7VP2Q=

components/backend/internal/middleware/auth_test.go

@@ -10,7 +10,9 @@ import (
 	"github.com/golang-jwt/jwt/v5"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
+	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/metadata"
+	"google.golang.org/grpc/status"
 
 	"github.com/swissdatasciencecenter/hackagon/components/backend/internal/config"
 	"github.com/swissdatasciencecenter/hackagon/components/backend/internal/middleware"
@@ -84,6 +86,7 @@ var _ = Describe("Auth Middleware", func() {
 			_, err := validator.AuthFunc()(ctx)
 			Expect(err).To(HaveOccurred())
 			Expect(err.Error()).To(ContainSubstring("token is malformed"))
+			Expect(status.Code(err)).To(Equal(codes.Unauthenticated))
 		})
 
 		It("rejects expired tokens", func() {
@@ -96,6 +99,7 @@ var _ = Describe("Auth Middleware", func() {
 			_, err := validator.AuthFunc()(ctx)
 			Expect(err).To(HaveOccurred())
 			Expect(err.Error()).To(ContainSubstring("token is expired"))
+			Expect(status.Code(err)).To(Equal(codes.Unauthenticated))
 		})
 
 		It("rejects tokens with future not-before", func() {
@@ -119,6 +123,21 @@ var _ = Describe("Auth Middleware", func() {
 			_, err = validator.AuthFunc()(ctx)
 			Expect(err).To(HaveOccurred())
 			Expect(err.Error()).To(ContainSubstring("token is not valid yet"))
+			Expect(status.Code(err)).To(Equal(codes.Unauthenticated))
+		})
+
+		It("rejects a token from the wrong issuer", func() {
+			tokenString := testutils.GenerateTestToken(
+				"wrong-issuer-user", 24*time.Hour, "https://not-our-issuer.example",
+			)
+			ctx := metadata.NewIncomingContext(context.Background(), metadata.Pairs(
+				"authorization", "Bearer "+tokenString,
+			))
+
+			validator := middleware.NewTestJWTValidator(cfg, keyfunc)
+			_, err := validator.AuthFunc()(ctx)
+			Expect(err).To(HaveOccurred())
+			Expect(status.Code(err)).To(Equal(codes.Unauthenticated))
 		})
 	})
 
@@ -158,6 +177,7 @@ var _ = Describe("Auth Middleware", func() {
 			_, err := validator.AuthFunc()(ctx)
 			Expect(err).To(HaveOccurred())
 			Expect(err.Error()).To(ContainSubstring("token is malformed"))
+			Expect(status.Code(err)).To(Equal(codes.Unauthenticated))
 		})
 	})
 

components/backend/internal/middleware/errors.go

@@ -4,7 +4,7 @@ import (
 	"errors"
 	"strings"
 
-	"github.com/golang-jwt/jwt/v4"
+	"github.com/golang-jwt/jwt/v5"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 )
@@ -35,25 +35,40 @@ func handleError(err error, setErrorCodes bool) error {
 	return err
 }
 
+// handleJwtError translates a failure from the jwt library into one of this
+// package's own errors, which `setGrpcErrorCodes` then turns into
+// `Unauthenticated`.
+//
+// Matched with `errors.Is` against the library's sentinel values. v5 reports a
+// bad token by wrapping those sentinels; it has no error *type* to match on —
+// the `*jwt.ValidationError` of v4 was removed. This file went on matching the
+// v4 type while `auth.go` parsed with v5, so nothing here ever matched and
+// every expired, malformed or badly signed token fell through to `Internal`,
+// reported to callers as a server fault rather than a rejected login.
+//
+// Anything unrecognised is returned untouched, so an error raised by `auth.go`
+// itself (a missing header, an unknown key id) reaches `setGrpcErrorCodes` as
+// the sentinel it already is.
 func handleJwtError(errIn error) error {
-	var err *jwt.ValidationError
-	if errors.As(errIn, &err) {
-		switch {
-		case err.Is(jwt.ErrTokenExpired):
-			return ErrTokenExpired
-		case err.Is(jwt.ErrTokenMalformed):
-			return ErrTokenMalformed
-		case err.Is(jwt.ErrTokenNotValidYet):
-			return ErrTokenNotValidYet
-		case err.Is(jwt.ErrTokenUsedBeforeIssued):
-			return ErrTokenUsedBeforeIssued
-		case err.Is(jwt.ErrTokenSignatureInvalid):
-			if strings.Contains(err.Error(), "signing method") {
-				return ErrBadAlgorithm
-			}
-
-			return ErrSignatureInvalid
+	switch {
+	case errors.Is(errIn, jwt.ErrTokenExpired):
+		return ErrTokenExpired
+	case errors.Is(errIn, jwt.ErrTokenMalformed):
+		return ErrTokenMalformed
+	case errors.Is(errIn, jwt.ErrTokenNotValidYet):
+		return ErrTokenNotValidYet
+	case errors.Is(errIn, jwt.ErrTokenUsedBeforeIssued):
+		return ErrTokenUsedBeforeIssued
+	case errors.Is(errIn, jwt.ErrTokenInvalidIssuer):
+		return ErrTokenInvalidIssuer
+	case errors.Is(errIn, jwt.ErrTokenSignatureInvalid):
+		// `WithValidMethods` rejects a wrong algorithm through the same
+		// sentinel, and only the message tells the two apart.
+		if strings.Contains(errIn.Error(), "signing method") {
+			return ErrBadAlgorithm
 		}
+
+		return ErrSignatureInvalid
 	}
 
 	return errIn

components/frontend/src/routes/(public)/invite/[token]/+page.server.ts

@@ -87,17 +87,13 @@ function authorizedFor(session: CustomSession | null) {
  * refusal falls back to the anonymous call: everything but that one field is
  * identical, and the page's whole point is being readable before signing in.
  * `usableSession` already screens out the refusal Auth.js reports, but a token
- * can also lapse between refreshes, and the backend answers that with INTERNAL
- * rather than UNAUTHENTICATED — see `TODO(backend: jwt-error-codes)` below.
+ * can also lapse between refreshes, and that arrives here as UNAUTHENTICATED.
  */
 function askPreview(token: string, grpc?: AuthorizedGrpc) {
   if (!grpc) return publicHackathonClient().previewInvite({ token })
 
   return grpc.hackathon.previewInvite({ token }).catch((e) => {
-    if (
-      e instanceof ClientError &&
-      (e.code === Status.UNAUTHENTICATED || e.code === Status.INTERNAL)
-    ) {
+    if (e instanceof ClientError && e.code === Status.UNAUTHENTICATED) {
       return publicHackathonClient().previewInvite({ token })
     }
     throw e
@@ -263,22 +259,22 @@ export const actions: Actions = {
           return fail(400, {
             message: e.details || "Some answers are not valid.",
           })
-        // The backend refusing the token itself. UNAUTHENTICATED is what the
-        // middleware means to send.
-        //
-        // TODO(backend: jwt-error-codes): INTERNAL is in this branch because it
-        // is what actually arrives. `errors.go` matches jwt/**v4**'s
-        // `*ValidationError` while `auth.go` parses with **v5**, which removed
-        // that type — so `errors.As` never matches and every auth failure falls
-        // past the `unauthenticatedErrors` list to `codes.Internal`. Drop
-        // INTERNAL from here once that is fixed; until then a genuine server
-        // fault during a join is reported to the user as an expired session,
-        // which is the lesser of the two wrong answers available.
-        if (e.code === Status.UNAUTHENTICATED || e.code === Status.INTERNAL)
+        // The backend refusing the token itself.
+        if (e.code === Status.UNAUTHENTICATED)
           return fail(401, {
             message:
               "Your sign-in has expired. Sign in again — this invitation still works.",
           })
+        // A real server fault, and now distinguishable from one: INTERNAL used
+        // to arrive for every rejected token too, so this page had to read it
+        // as an expired session. It no longer does, so a fault can say so and
+        // the invitation can be described as still good — which it is.
+        if (e.code === Status.INTERNAL)
+          return fail(500, {
+            message:
+              "Something went wrong on our side. Your invitation is still " +
+              "valid — please try again.",
+          })
       }
       throw e
     }