Skip to content

Conversation

alexluong
Copy link
Collaborator

No description provided.

Comment on lines +20 to +31
if err != nil {
// TODO: Consider sending a more detailed error message.
// Currently we don't have clear specs on how to send back error message.
c.AbortWithStatus(http.StatusUnauthorized)
return
}
if authorizationToken != apiKey {
// TODO: Consider sending a more detailed error message.
// Currently we don't have clear specs on how to send back error message.
c.AbortWithStatus(http.StatusUnauthorized)
return
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to point this out as the spec hasn't mentioned errors yet, afaik. Do you want to circle back on this later?

Happy to sort this out now if you can share what you have in mind or provide the OpenAPI schema spec or something like that.

func setupRouter(apiKey string) *gin.Engine {
gin.SetMode(gin.TestMode)
r := gin.Default()
r.Use(apiKeyAuthMiddleware(apiKey))
Copy link
Collaborator Author

@alexluong alexluong Aug 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sooo in the spec I saw you mentioned JWT token but I'm not super super clear on that just yet.

My understanding:

  • optional API key auth mechanism for internal purposes
  • JWT auth mechanism for portal / per-tenant usage

Please correct me if there's any misunderstanding here. Will tackle the Tenant API now and implement the JWT mechanism in another PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's correct, we'll need to think through how that works since the tenant is implied with the JWT really the endpoint wouldn't be have /:tenant_id

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's more of a API design concern, because if you'd like to remove the /:tenant_id path param we can get the tenant ID from the JWT tokentoo.

@alexluong alexluong changed the title feat: Authentication middleware feat: API key authentication middleware Aug 28, 2024
func setupRouter(apiKey string) *gin.Engine {
gin.SetMode(gin.TestMode)
r := gin.Default()
r.Use(apiKeyAuthMiddleware(apiKey))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's correct, we'll need to think through how that works since the tenant is implied with the JWT really the endpoint wouldn't be have /:tenant_id

Base automatically changed from testing to main September 6, 2024 15:45
# Conflicts:
#	internal/config/config.go
#	internal/services/api/router.go
@alexluong alexluong merged commit c5944c7 into main Sep 6, 2024
@alexluong alexluong deleted the auth branch September 6, 2024 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants