User Authentication and Authorization using JWT #2
Conversation
…d necessary comments
db/user.go
Outdated
| Country string `db:"country" json:"country"` | ||
| State string `db:"state" json:"state"` | ||
| City string `db:"city" json:"city"` | ||
| CreatedAt string `db:"created_at" json:"created_at"` |
There was a problem hiding this comment.
can we use time.Time instead of string?
There was a problem hiding this comment.
Actually we are thinking of removing it since we are neither accepting it from the frontend nor we are sending it back.
service/user_http.go
Outdated
| return | ||
| } | ||
|
|
||
| user, err1 := deps.Store.GetUser(req.Context(), int(userID)) |
There was a problem hiding this comment.
can we rename it to err instead of err1?
service/user_http.go
Outdated
| func getUserHandler(deps Dependencies) http.HandlerFunc { | ||
| return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { | ||
| //fetch usedId from request | ||
| authToken := req.Header["Token"] |
There was a problem hiding this comment.
there is a Get method on this Header map, can we use that instead of accessing it directly
| } | ||
|
|
||
| //generateJWT function generates and return a new JWT token | ||
| func generateJwt(userID int) (tokenString string, err error) { |
There was a problem hiding this comment.
IMO, Naked returns are okay for smaller functions.. however we should avoid them for longer functions..
There was a problem hiding this comment.
Yes Indeed, Will change this in the next commit.
service/session_http.go
Outdated
| } | ||
|
|
||
| respBytes, err := json.Marshal(authbody) | ||
| if err != nil { |
There was a problem hiding this comment.
Generally json.Marshal doesn't fail.. however we shouldn't ignore this error..
can we return 5xx from here?
service/session_http.go
Outdated
| err = deps.Store.CreateBlacklistedToken(req.Context(), userBlackListedToken) | ||
| if err != nil { | ||
| ae.Error(ae.ErrFailedToCreate, "Error creating blaclisted token record", err) | ||
| rw.Header().Add("Content-Type", "application/json") |
There was a problem hiding this comment.
can we use Header.Set method instead of Header.Add?
because multiple Content-Type doesn't make sense here..
Set method replaces the value of existing.. and Add method appends the value..
service/session_http.go
Outdated
| authToken := req.Header["Token"] | ||
|
|
||
| //fetching details from the token | ||
| userID, expirationTimeStamp, err := getDataFromToken(authToken[0]) |
There was a problem hiding this comment.
Here, authToken[0] will be index out of range if we don't pass the Token in header.. can we fix this?
or we can use Header.Get method
3 participants
Tasks Completed :