Skip to content
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

Add JWT authentication #91

Open
sagikazarmark opened this issue Jul 26, 2019 · 4 comments
Open

Add JWT authentication #91

sagikazarmark opened this issue Jul 26, 2019 · 4 comments
Labels
enhancement New feature or request

Comments

@sagikazarmark
Copy link
Owner

Keep the current TODO API for simplicity? Or create a new API with protected resources?

@sagikazarmark sagikazarmark added the enhancement New feature or request label Jul 26, 2019
@atomicleads
Copy link

I'm trying to implement JWT auth using this beautiful starter project, but got into troubles. Whenever i need to protect all endpoints, go-kit provided middleware working just fine.
I do something like this:

endpointMiddleware := []endpoint.Middleware{
		correlation.Middleware(),
		jwt.NewParser(jwtKeyFunc, stdjwt.SigningMethodHS256, jwt.StandardClaimsFactory), // JWT parser endpoint
	}
endpoints := somedriver.TraceEndpoints(somedriver.MakeEndpoints(
			service,
			kitxendpoint.Chain(endpointMiddleware...),
			appkit.EndpointLogger(logger),
		))

But how do i protect only few endpoints? For example users service where registration / login should not be protected by jwt auth. If i skip jwt token i'm getting pretty non informative response and log message
token up for parsing was not passed through the context

{
  "type": "about:blank",
  "title": "Internal Server Error",
  "status": 500,
  "detail": "something went wrong"
}

Could you please suggest better way of implementing jwt ?

@sagikazarmark
Copy link
Owner Author

You need two sets of endpointMiddlewares. One with auth, one without. Same for transport level middlewares.

The 500 error is because JWT errors are returned in the endpoint layer and the generic ErrorEncoder doesn't know how to handle does, so it falls back to 500 error.

Personally I think this is the biggest flaw in go-kit at the moment. I explained this issue in details here: go-kit/kit#923

I don't have an immediate answer to this issue. The best you can do at the moment is writing your own error encoder that knows these error types: https://github.com/go-kit/kit/blob/492a87e/auth/jwt/middleware.go#L24-L45

That way you can return 401 properly.

I plan come up with a solution, I have a few ideas actually, but none of them make me perfectly happy.

@atomicleads
Copy link

That means all errors you are returning in endpoints will result as the same generic ErrorEncoder message. Example:

// MakeCreateTodoEndpoint returns an endpoint for the matching method of the underlying service.
func MakeCreateTodoEndpoint(service todo.Service) endpoint.Endpoint {
	return func(ctx context.Context, request interface{}) (interface{}, error) {
		req := request.(createTodoRequest)

		id, err := service.CreateTodo(ctx, req.Text)

		return createTodoResponse{
			ID: id,
		}, err
	}
}

I tested my endpoints that i made same as yours and can confirm that.

@sagikazarmark
Copy link
Owner Author

That's correct. Sometimes you want that (eg. there are no business errors returned from your service), but that's not always the case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant