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

API keys support on the backend #379

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

titanventura
Copy link

@titanventura titanventura commented Feb 28, 2024

Fixes #360 partially

Changes made

  • Added a table for API Keys
  • added routes for users to be able to manage their API keys
  • added a middleware to authenticate routes using API keys
  • use the middleware for GET method of /api/watched endpoint

…ect /api/watched endpoint using the APIKeyMiddleware
Copy link
Member

@IRHM IRHM left a comment

Choose a reason for hiding this comment

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

Hey @titanventura, thanks for making a PR (and sorry for taking so long getting to it)!

I had a quick look and noticed some changes we could make, let me know if you have any thoughts on the feedback.

I noticed the APIKeyMiddlewareWrapper, did adding the header check to the AuthRequired middleware not work out? I think that way would be preferable, just need to move the x-api-key header check into AuthRequired and if we find it, we can process it as an API key instead (in APIKeyMiddlewareWrapper, if you'd like, but probably renamed).

I know this is a draft, so you may have been getting to this, but the route handlers seem to have all the logic in them. We could separate most of it out to 'service' functions.

Thanks again for taking the time!

server/apikeys.go Outdated Show resolved Hide resolved
server/apikeys.go Outdated Show resolved Hide resolved
server/apikeys.go Outdated Show resolved Hide resolved
server/apikeys.go Outdated Show resolved Hide resolved
ID uint `gorm:"primarykey" json:"id"`
CreatedAt time.Time `json:"createdAt"`
Value string `gorm:"not null" json:"value"`
UserID uint `gorm:"not null" json:"-"`
Copy link
Member

Choose a reason for hiding this comment

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

We may want to make this a foreign key that maps to a User to ensure data integrity.

"gorm.io/gorm"
)

type ApiKey struct {
Copy link
Member

Choose a reason for hiding this comment

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

An UpdatedAt field might be worth adding in here too.

@titanventura
Copy link
Author

@IRHM , please go through the updated PR. Thanks !

@titanventura titanventura requested a review from IRHM March 2, 2024 20:06
@IRHM IRHM added the enhancement New feature or request label Mar 6, 2024
IRHM
IRHM previously approved these changes Mar 6, 2024
@IRHM
Copy link
Member

IRHM commented Mar 6, 2024

Hi @titanventura, I'm happy to get this merged. Thanks for the good work!

There are a few minor things, but It's probably best I do them after deciding how the UI will work so it isn't wasted effort incase it needs changing.

Thanks!

@titanventura titanventura marked this pull request as ready for review June 5, 2024 16:08
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

Successfully merging this pull request may close these issues.

API Keys for users - giving other apps access to users data securely
2 participants