-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Feature/jwt auth #1895
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.
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
base: development
Are you sure you want to change the base?
Feature/jwt auth #1895
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This example seems a bit incomplete to me. Adding the comments for support of the request header is fine but we need to also figure workarounds that we can use. An example should either be added with complete functionality working or if not then a working solution people can use.
Also it lacks test files. We should write tests for new code else our code coverage will decrease.
if username == "" { | ||
// Respond with HTTP 400 if username is missing | ||
ctx.Error(http.StatusBadRequest, "username is required to generate a token") | ||
return nil, nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of using gofr.NewHTTPError(http.StatusBadRequest, "username is required to generate a token"), we should leverage the existing ErrorMissingParam
type for consistency and better error handling.
This will ensure that the error response aligns with the framework's standard structure for missing parameters. Currently we are getting status 200 even for this request.
tokenStr, err := token.SignedString([]byte(secret)) | ||
if err != nil { | ||
ctx.Error(http.StatusInternalServerError, "Failed to generate JWT token") | ||
return nil, nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the token signing fails, it is more indicative of an authorization issue rather than a server error. Consider using http.StatusUnauthorized to better reflect the nature of the error.
And also we need to return appropriate error from here so we get the correct response codes.
"os" | ||
|
||
"gofr.dev/pkg/gofr" | ||
"gofr.dev/examples/using-jwt-auth/handler" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sort the imports here please
// This will only work once GoFr supports header access in middleware. | ||
func SecureHandler(ctx *gofr.Context) (interface{}, error) { | ||
// When header access is supported, the middleware will inject user info into context. | ||
user := ctx.Context.Value("user") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of directly accessing ctx.Context.Value("user")
, consider using ctx.GetAuthInfo().GetClaims()
to retrieve the claims information. This approach is more structured, aligns with the framework's design, and avoids directly interacting with the raw context values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure let me update the code and will send pr
--- updated-dependencies: - dependency-name: go.opentelemetry.io/otel/trace dependency-version: 1.37.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]>
… access supported)
b8ace39
to
ebe07b4
Compare
can u review this once and i will make further enhancements and update readme as well @Umang01-hash |
Description
examples/using-jwt-auth/
that demonstrates how to integrate JWT-based authentication in a GoFr application./login
route that returns a signed JWT for the given user.middleware/jwt.go
that illustrates how JWT validation would work once header access is available in GoFr.secure.go
file that shows a protected route using the JWT middleware.README.md
with detailed instructions on setup, structure, usage, security best practices, and future support.Breaking Changes
This is a self-contained example and does not modify or affect any existing framework components.
Additional Information
github.com/golang-jwt/jwt/v5
for token creation and parsing.JWT_SECRET
environment variable, with a fallback for local development.README.md
includes example code, explanations, and security notes to help users understand and extend the implementation.