Move Firebase auth into OpenAPI validator#23
Conversation
There was a problem hiding this comment.
Pull request overview
This PR moves Firebase ID token verification into the OpenAPI request validator’s AuthenticationFunc, so authentication failures flow through the validator’s error handling while still making validated tokens available on Gin/request contexts for downstream handlers.
Changes:
- Added
FirebaseAuthenticationFuncfor kin-openapi’sopenapi3filter.AuthenticationFunc, verifying Firebase tokens and attaching them to contexts. - Centralized token parsing/verification logic via shared helpers (
verifyFirebaseToken,setFirebaseToken) and added a mechanism to pass auth failure status/message to the validatorErrorHandler. - Updated server wiring to use
OapiRequestValidatorWithOptionswith a customErrorHandlerandAuthenticationFunc, removing the explicitFirebaseAuthmiddleware from the chain.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| internal/middleware/firebase_auth.go | Introduces OpenAPI validator auth integration, shared verification helpers, and auth-error propagation to Gin context. |
| cmd/server/main.go | Switches to OpenAPI validator options with custom error handling + Firebase authentication function. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if err != nil { | ||
| var authErr *AuthenticationError | ||
| if errors.As(err, &authErr) { | ||
| c.AbortWithStatusJSON(authErr.StatusCode, gin.H{"error": authErr.Message}) | ||
| return | ||
| } | ||
| c.AbortWithStatusJSON(http.StatusUnauthorized, gin.H{"error": "Authentication failed"}) | ||
| return |
There was a problem hiding this comment.
The fallback branch that returns a generic 401 ("Authentication failed") is effectively unreachable right now because verifyFirebaseToken always returns an *AuthenticationError on failure. Consider removing the dead branch, or change verifyFirebaseToken to return non-auth errors distinctly (e.g., wrap unexpected errors as 500) so this branch can be meaningful.
| } | ||
|
|
||
| // FirebaseAuth は Authorization: Bearer <Firebase ID Token> を検証する Gin ミドルウェアです。 | ||
| // 検証に成功すると、デコードされたトークン（*auth.Token）を context に格納して次のハンドラに渡します。 |
There was a problem hiding this comment.
FirebaseAuth middleware is no longer referenced anywhere in this repository (the server now wires auth via the OpenAPI validator). Consider removing this middleware or clearly deprecating it to avoid confusion about which auth path is active.
| // 検証に成功すると、デコードされたトークン（*auth.Token）を context に格納して次のハンドラに渡します。 | |
| // 検証に成功すると、デコードされたトークン（*auth.Token）を context に格納して次のハンドラに渡します。 | |
| // Deprecated: このミドルウェアは現在サーバでは使用されていません。認証は OpenAPI validator を通じて | |
| // FirebaseAuthenticationFunc を用いて行われます。新しいコードでは FirebaseAuth ではなく | |
| // FirebaseAuthenticationFunc を利用してください。 |
| StatusCode: http.StatusUnauthorized, | ||
| Message: "Authentication context is unavailable", |
There was a problem hiding this comment.
When GetGinContext(ctx) returns nil, this indicates a server/middleware wiring issue rather than an authentication failure. Returning 401 here can mislead clients and hide operational problems; consider treating this as an internal error (e.g., 500) and/or ensuring the error handler does not expose this internal message to clients.
| StatusCode: http.StatusUnauthorized, | |
| Message: "Authentication context is unavailable", | |
| StatusCode: http.StatusInternalServerError, | |
| Message: "Internal authentication error", |
Summary
Testing