From 362a8826b93f58714414ed87f2d87d9b1c8cff2b Mon Sep 17 00:00:00 2001 From: Kartik Ohri Date: Sat, 3 May 2025 15:02:23 +0530 Subject: [PATCH 1/6] Implement OpenSubsonic's API Key authentication Ref: https://opensubsonic.netlify.app/docs/extensions/apikeyauth/. Add functionality for users to create, manage, and use API keys for authentication. * **Database:** * Added a new `api_key`(`20250415111500_add_api_key_table.go`) table to store API keys associated with users. * **Backend:** * Defined the `APIKey` model. * Implemented `APIKeyRepository` for database operations. * Added `FindByAPIKey` method to `UserRepository` to allow user lookup via API key. * Updated Subsonic API error responses to include API key-related errors and messaging. * Integrated API key authentication into the subsonic authentication middleware. * **UI:** * Added new React components for API Key CRUD operations: `ApiKeyList`, `ApiKeyCreate`, and `ApiKeyEdit`. * The API Key management section can be accessed from the settings menu. # Conflicts: # model/user.go # server/subsonic/middlewares.go # server/subsonic/middlewares_test.go # tests/mock_data_store.go # tests/mock_user_repo.go --- .../20250415111500_add_api_key_table.go | 39 ++++ model/api_key.go | 26 +++ model/datastore.go | 1 + model/user.go | 2 + persistence/api_key_repository.go | 158 +++++++++++++ persistence/api_key_repository_test.go | 216 ++++++++++++++++++ persistence/persistence.go | 6 + persistence/user_repository.go | 12 + server/nativeapi/native_api.go | 1 + server/subsonic/middlewares.go | 66 +++++- server/subsonic/middlewares_test.go | 59 +++++ server/subsonic/responses/errors.go | 38 +-- tests/mock_apikey_repo.go | 57 +++++ tests/mock_data_store.go | 15 +- tests/mock_user_repo.go | 29 ++- ui/src/App.jsx | 6 + ui/src/apikey/ApiKeyCreate.jsx | 57 +++++ ui/src/apikey/ApiKeyEdit.jsx | 75 ++++++ ui/src/apikey/ApiKeyList.jsx | 58 +++++ ui/src/apikey/index.js | 11 + ui/src/i18n/en.json | 16 ++ ui/src/layout/AppBar.jsx | 10 + ui/src/layout/Menu.jsx | 5 +- 23 files changed, 931 insertions(+), 32 deletions(-) create mode 100644 db/migrations/20250415111500_add_api_key_table.go create mode 100644 model/api_key.go create mode 100644 persistence/api_key_repository.go create mode 100644 persistence/api_key_repository_test.go create mode 100644 tests/mock_apikey_repo.go create mode 100644 ui/src/apikey/ApiKeyCreate.jsx create mode 100644 ui/src/apikey/ApiKeyEdit.jsx create mode 100644 ui/src/apikey/ApiKeyList.jsx create mode 100644 ui/src/apikey/index.js diff --git a/db/migrations/20250415111500_add_api_key_table.go b/db/migrations/20250415111500_add_api_key_table.go new file mode 100644 index 000000000..d75ddd19f --- /dev/null +++ b/db/migrations/20250415111500_add_api_key_table.go @@ -0,0 +1,39 @@ +package migrations + +import ( + "context" + "database/sql" + + "github.com/pressly/goose/v3" +) + +func init() { + goose.AddMigrationContext(upAddApiKeyTable, downAddApiKeyTable) +} + +func upAddApiKeyTable(ctx context.Context, tx *sql.Tx) error { + _, err := tx.ExecContext(ctx, ` + create table if not exists api_key ( + id text not null primary key, + user_id text not null, + name text not null, + key text not null unique, + created_at datetime not null, + + foreign key (user_id) + references user(id) + on delete cascade + ); + + create index if not exists api_key_key on api_key(key); + create index if not exists api_key_user_id on api_key(user_id); +`) + return err +} + +func downAddApiKeyTable(ctx context.Context, tx *sql.Tx) error { + _, err := tx.ExecContext(ctx, ` + drop table api_key; +`) + return err +} diff --git a/model/api_key.go b/model/api_key.go new file mode 100644 index 000000000..6d238f422 --- /dev/null +++ b/model/api_key.go @@ -0,0 +1,26 @@ +package model + +import ( + "github.com/deluan/rest" + "time" +) + +type APIKey struct { + ID string `structs:"id" json:"id"` + UserID string `structs:"user_id" json:"userId"` + Name string `structs:"name" json:"name"` + Key string `structs:"key" json:"key"` + CreatedAt time.Time `structs:"created_at" json:"createdAt"` +} + +type APIKeys []APIKey + +type APIKeyRepository interface { + ResourceRepository + rest.Persistable + CountAll(...QueryOptions) (int64, error) + Get(id string) (*APIKey, error) + GetAll(options ...QueryOptions) (APIKeys, error) + Put(*APIKey) error + FindByKey(key string) (*APIKey, error) +} diff --git a/model/datastore.go b/model/datastore.go index 4290e2134..ebd99e25b 100644 --- a/model/datastore.go +++ b/model/datastore.go @@ -38,6 +38,7 @@ type DataStore interface { User(ctx context.Context) UserRepository UserProps(ctx context.Context) UserPropsRepository ScrobbleBuffer(ctx context.Context) ScrobbleBufferRepository + APIKey(ctx context.Context) APIKeyRepository Resource(ctx context.Context, model interface{}) ResourceRepository diff --git a/model/user.go b/model/user.go index aabedc096..042ac8d60 100644 --- a/model/user.go +++ b/model/user.go @@ -56,4 +56,6 @@ type UserRepository interface { // Library association methods GetUserLibraries(userID string) (Libraries, error) SetUserLibraries(userID string, libraryIDs []int) error + // FindByAPIKey finds a user by the provided API key + FindByAPIKey(key string) (*User, error) } diff --git a/persistence/api_key_repository.go b/persistence/api_key_repository.go new file mode 100644 index 000000000..583578d3c --- /dev/null +++ b/persistence/api_key_repository.go @@ -0,0 +1,158 @@ +package persistence + +import ( + "context" + "time" + + "github.com/deluan/rest" + + . "github.com/Masterminds/squirrel" + "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/model/id" + "github.com/pocketbase/dbx" +) + +type apiKeyRepository struct { + sqlRepository +} + +func NewAPIKeyRepository(ctx context.Context, db dbx.Builder) model.APIKeyRepository { + r := &apiKeyRepository{} + r.ctx = ctx + r.db = db + r.registerModel(&model.APIKey{}, nil) + return r +} + +func (r *apiKeyRepository) userFilter() Sqlizer { + user := loggedUser(r.ctx) + if user.IsAdmin { + return And{} + } + return Eq{"user_id": user.ID} +} + +func (r *apiKeyRepository) CountAll(options ...model.QueryOptions) (int64, error) { + sq := Select().From(r.tableName).Where(r.userFilter()) + return r.count(sq, options...) +} + +func (r *apiKeyRepository) Get(id string) (*model.APIKey, error) { + sel := r.newSelect().Columns("*").Where(And{Eq{"id": id}}) + var res model.APIKey + err := r.queryOne(sel, &res) + if err != nil { + return nil, err + } + return &res, err +} + +func (r *apiKeyRepository) GetAll(options ...model.QueryOptions) (model.APIKeys, error) { + sel := r.newSelect(options...).Columns("*").Where(r.userFilter()) + res := model.APIKeys{} + err := r.queryAll(sel, &res) + if err != nil { + return nil, err + } + return res, err +} + +func (r *apiKeyRepository) Put(ak *model.APIKey) error { + if ak.ID == "" { + ak.ID = id.NewRandom() + } + ak.CreatedAt = time.Now() + values, err := toSQLArgs(*ak) + if err != nil { + return err + } + insert := Insert(r.tableName).SetMap(values) + _, err = r.executeSQL(insert) + return err +} + +func (r *apiKeyRepository) Count(options ...rest.QueryOptions) (int64, error) { + return r.CountAll(r.parseRestOptions(r.ctx, options...)) +} + +func (r *apiKeyRepository) Read(id string) (interface{}, error) { + user := loggedUser(r.ctx) + apiKey, err := r.Get(id) + if err != nil { + return nil, err + } + if !user.IsAdmin && apiKey.UserID != user.ID { + return nil, rest.ErrPermissionDenied + } + return apiKey, err +} + +func (r *apiKeyRepository) ReadAll(options ...rest.QueryOptions) (interface{}, error) { + return r.GetAll(r.parseRestOptions(r.ctx, options...)) +} + +func (r *apiKeyRepository) EntityName() string { + return "apikey" +} + +func (r *apiKeyRepository) NewInstance() interface{} { + return &model.APIKey{} +} + +func (r *apiKeyRepository) Save(entity interface{}) (string, error) { + ak := entity.(*model.APIKey) + user := loggedUser(r.ctx) + ak.UserID = user.ID + // prefix API keys with nav_ + ak.Key = "nav_" + id.NewRandom() + err := r.Put(ak) + if err != nil { + return "", err + } + return ak.ID, err +} + +func (r *apiKeyRepository) Update(id string, entity interface{}, _ ...string) error { + ak := entity.(*model.APIKey) + current, err := r.Get(id) + if err != nil { + return err + } + user := loggedUser(r.ctx) + if !user.IsAdmin && current.UserID != user.ID { + return rest.ErrPermissionDenied + } + + // Only allow updating name + update := Update(r.tableName). + Set("name", ak.Name). + Where(Eq{"id": id}) + _, err = r.executeSQL(update) + return err +} + +func (r *apiKeyRepository) Delete(id string) error { + user := loggedUser(r.ctx) + apiKey, err := r.Get(id) + if err != nil { + return err + } + if !user.IsAdmin && apiKey.UserID != user.ID { + return rest.ErrPermissionDenied + } + return r.delete(Eq{"id": id}) +} + +func (r *apiKeyRepository) FindByKey(key string) (*model.APIKey, error) { + sel := r.newSelect().Columns("*").Where(Eq{"key": key}) + var res model.APIKey + err := r.queryOne(sel, &res) + if err != nil { + return nil, err + } + return &res, err +} + +var _ model.APIKeyRepository = (*apiKeyRepository)(nil) +var _ rest.Repository = (*apiKeyRepository)(nil) +var _ rest.Persistable = (*apiKeyRepository)(nil) diff --git a/persistence/api_key_repository_test.go b/persistence/api_key_repository_test.go new file mode 100644 index 000000000..44ab6f0cc --- /dev/null +++ b/persistence/api_key_repository_test.go @@ -0,0 +1,216 @@ +package persistence + +import ( + "context" + "github.com/deluan/rest" + "github.com/navidrome/navidrome/log" + "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/model/request" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("APIKeyRepository", func() { + var repo model.APIKeyRepository + + BeforeEach(func() { + ctx := log.NewContext(context.TODO()) + ctx = request.WithUser(ctx, model.User{ID: "userid", UserName: "userid", IsAdmin: true}) + repo = NewAPIKeyRepository(ctx, GetDBXBuilder()) + }) + + Describe("Put", func() { + It("sets an ID if it is not set", func() { + apiKey := &model.APIKey{ + UserID: "userid", + Name: "Test API Key", + Key: "test-key", + } + + err := repo.Put(apiKey) + + Expect(err).ToNot(HaveOccurred()) + Expect(apiKey.ID).ToNot(BeEmpty()) + Expect(apiKey.CreatedAt).ToNot(BeZero()) + }) + + It("keeps existing values", func() { + apiKey := &model.APIKey{ + ID: "existing-id", + UserID: "userid", + Name: "Test API Key 2", + Key: "test-key-2", + } + + err := repo.Put(apiKey) + + Expect(err).ToNot(HaveOccurred()) + Expect(apiKey.ID).To(Equal("existing-id")) + Expect(apiKey.CreatedAt).ToNot(BeZero()) + }) + }) + + Describe("FindByKey", func() { + It("returns the API key with matching key", func() { + apiKey := &model.APIKey{ + UserID: "userid", + Name: "Unique API Key", + Key: "unique-test-key", + } + + err := repo.Put(apiKey) + Expect(err).ToNot(HaveOccurred()) + + result, err := repo.FindByKey("unique-test-key") + + Expect(err).ToNot(HaveOccurred()) + Expect(result.ID).To(Equal(apiKey.ID)) + Expect(result.Key).To(Equal("unique-test-key")) + }) + + It("returns error when key not found", func() { + _, err := repo.FindByKey("non-existent-key") + Expect(err).To(HaveOccurred()) + }) + }) + + Describe("Save", func() { + It("creates a new API key with a generated key", func() { + apiKey := &model.APIKey{ + Name: "Test API Key Save", + } + + id, err := repo.Save(apiKey) + + Expect(err).ToNot(HaveOccurred()) + Expect(id).ToNot(BeEmpty()) + Expect(apiKey.Key).To(HavePrefix("nav_")) + Expect(apiKey.UserID).To(Equal("userid")) + }) + }) + + Describe("Update", func() { + It("only updates the name field", func() { + apiKey := &model.APIKey{ + UserID: "userid", + Name: "Original Name", + Key: "test-key-for-update", + } + + err := repo.Put(apiKey) + Expect(err).ToNot(HaveOccurred()) + + updateKey := &model.APIKey{ + Name: "Updated Name", + Key: "should-not-change", + UserID: "2222", + } + + err = repo.Update(apiKey.ID, updateKey) + Expect(err).ToNot(HaveOccurred()) + + result, err := repo.Get(apiKey.ID) + Expect(err).ToNot(HaveOccurred()) + Expect(result.Name).To(Equal("Updated Name")) + Expect(result.Key).To(Equal("test-key-for-update")) + Expect(result.UserID).To(Equal("userid")) + }) + + It("returns error when attempting to update non-existent key", func() { + err := repo.Update("non-existent-id", &model.APIKey{Name: "Updated Name"}) + Expect(err).To(HaveOccurred()) + }) + }) + + Describe("Delete", func() { + It("deletes an existing API key", func() { + apiKey := &model.APIKey{ + UserID: "userid", + Name: "API Key to Delete", + Key: "key-to-delete", + } + + err := repo.Put(apiKey) + Expect(err).ToNot(HaveOccurred()) + + err = repo.Delete(apiKey.ID) + Expect(err).ToNot(HaveOccurred()) + + _, err = repo.Get(apiKey.ID) + Expect(err).To(HaveOccurred()) + }) + }) + + Describe("User permissions", func() { + var nonAdminCtx context.Context + var nonAdminRepo model.APIKeyRepository + var adminKey model.APIKey + + BeforeEach(func() { + nonAdminCtx = log.NewContext(context.TODO()) + nonAdminCtx = context.WithValue(nonAdminCtx, "user", model.User{ID: "2222", UserName: "user", IsAdmin: false}) + nonAdminRepo = NewAPIKeyRepository(nonAdminCtx, GetDBXBuilder()) + + cleanupKeys := func(key string) { + foundKey, err := repo.FindByKey(key) + if err == nil { + _ = repo.Delete(foundKey.ID) + } + } + cleanupKeys("admin-key") + cleanupKeys("user-key") + + tmpAdminKey := &model.APIKey{ + UserID: "userid", + Name: "Admin's API Key", + Key: "admin-key", + } + err := repo.Put(tmpAdminKey) + Expect(err).ToNot(HaveOccurred()) + adminKey = *tmpAdminKey + + userKey := &model.APIKey{ + UserID: "2222", + Name: "User's API Key", + Key: "user-key", + } + err = repo.Put(userKey) + Expect(err).ToNot(HaveOccurred()) + }) + + It("non-admin users can only see their own API keys", func() { + results, err := nonAdminRepo.GetAll() + Expect(err).ToNot(HaveOccurred()) + + for _, key := range results { + Expect(key.UserID).To(Equal("2222")) + } + }) + + It("admin users can see all API keys", func() { + results, err := repo.GetAll() + Expect(err).ToNot(HaveOccurred()) + + userIds := make(map[string]bool) + for _, key := range results { + userIds[key.UserID] = true + } + + Expect(userIds).To(HaveKey("userid")) + Expect(userIds).To(HaveKey("2222")) + }) + + It("a user cannot view/delete/update another user's key", func() { + result, err := nonAdminRepo.Read(adminKey.ID) + Expect(result).To(BeNil()) + Expect(err).To(MatchError(rest.ErrPermissionDenied)) + + updatedKey := &model.APIKey{Name: "new admin key name"} + err = nonAdminRepo.Update(adminKey.ID, updatedKey) + Expect(err).To(MatchError(rest.ErrPermissionDenied)) + + err = nonAdminRepo.Delete(adminKey.ID) + Expect(err).To(MatchError(rest.ErrPermissionDenied)) + }) + }) +}) diff --git a/persistence/persistence.go b/persistence/persistence.go index ac607f85f..76f89e7ec 100644 --- a/persistence/persistence.go +++ b/persistence/persistence.go @@ -89,6 +89,10 @@ func (s *SQLStore) ScrobbleBuffer(ctx context.Context) model.ScrobbleBufferRepos return NewScrobbleBufferRepository(ctx, s.getDBXBuilder()) } +func (s *SQLStore) APIKey(ctx context.Context) model.APIKeyRepository { + return NewAPIKeyRepository(ctx, s.getDBXBuilder()) +} + func (s *SQLStore) Resource(ctx context.Context, m interface{}) model.ResourceRepository { switch m.(type) { case model.User: @@ -113,6 +117,8 @@ func (s *SQLStore) Resource(ctx context.Context, m interface{}) model.ResourceRe return s.Share(ctx).(model.ResourceRepository) case model.Tag: return s.Tag(ctx).(model.ResourceRepository) + case model.APIKey: + return s.APIKey(ctx).(model.ResourceRepository) } log.Error("Resource not implemented", "model", reflect.TypeOf(m).Name()) return nil diff --git a/persistence/user_repository.go b/persistence/user_repository.go index a7181b1a7..c675228d3 100644 --- a/persistence/user_repository.go +++ b/persistence/user_repository.go @@ -193,6 +193,18 @@ func (r *userRepository) FindByUsernameWithPassword(username string) (*model.Use return usr, nil } +func (r *userRepository) FindByAPIKey(key string) (*model.User, error) { + // find the API key in the database + apiKeyRepo := NewAPIKeyRepository(r.ctx, r.db) + apiKey, err := apiKeyRepo.FindByKey(key) + if err != nil { + return nil, err + } + + // Then get the user associated with this API key + return r.Get(apiKey.UserID) +} + func (r *userRepository) UpdateLastLoginAt(id string) error { upd := Update(r.tableName).Where(Eq{"id": id}).Set("last_login_at", time.Now()) _, err := r.executeSQL(upd) diff --git a/server/nativeapi/native_api.go b/server/nativeapi/native_api.go index 370bdbd1e..c6d6e9c64 100644 --- a/server/nativeapi/native_api.go +++ b/server/nativeapi/native_api.go @@ -55,6 +55,7 @@ func (n *Router) routes() http.Handler { n.R(r, "/transcoding", model.Transcoding{}, conf.Server.EnableTranscodingConfig) n.R(r, "/radio", model.Radio{}, true) n.R(r, "/tag", model.Tag{}, true) + n.R(r, "/apikey", model.APIKey{}, true) if conf.Server.EnableSharing { n.RX(r, "/share", n.share.NewRepository, true) } diff --git a/server/subsonic/middlewares.go b/server/subsonic/middlewares.go index af1ba448f..a39063273 100644 --- a/server/subsonic/middlewares.go +++ b/server/subsonic/middlewares.go @@ -66,11 +66,12 @@ func checkRequiredParameters(next http.Handler) http.Handler { username, _ := fromInternalOrProxyAuth(r) if username != "" { requiredParameters = []string{"v", "c"} + } else if apiKey, _ := p.String("apiKey"); apiKey != "" { + requiredParameters = []string{"v", "c"} } else { requiredParameters = []string{"u", "v", "c"} } - p := req.Params(r) for _, param := range requiredParameters { if _, err := p.String(param); err != nil { log.Warn(r, err) @@ -123,21 +124,62 @@ func authenticate(ds model.DataStore) func(next http.Handler) http.Handler { token, _ := p.String("t") salt, _ := p.String("s") jwt, _ := p.String("jwt") + apiKey, _ := p.String("apiKey") - usr, err = ds.User(ctx).FindByUsernameWithPassword(username) - if errors.Is(err, context.Canceled) { - log.Debug(ctx, "API: Request canceled when authenticating", "auth", "subsonic", "username", username, "remoteAddr", r.RemoteAddr, err) + // When an API key is provided, username should not be provided + if apiKey != "" && username != "" { + log.Warn(ctx, "API: Invalid login - username provided with API key", "auth", "subsonic", "remoteAddr", r.RemoteAddr) + sendError(w, r, newError(responses.ErrorMultipleAuthMechanismsProvided)) return } - switch { - case errors.Is(err, model.ErrNotFound): - log.Warn(ctx, "API: Invalid login", "auth", "subsonic", "username", username, "remoteAddr", r.RemoteAddr, err) - case err != nil: - log.Error(ctx, "API: Error authenticating username", "auth", "subsonic", "username", username, "remoteAddr", r.RemoteAddr, err) - default: - err = validateCredentials(usr, pass, token, salt, jwt) - if err != nil { + + // Check for conflicting authentication mechanisms + authMechanismsCount := 0 + if apiKey != "" { + authMechanismsCount++ + } + if pass != "" { + authMechanismsCount++ + } + if token != "" && salt != "" { + authMechanismsCount++ + } + if jwt != "" { + authMechanismsCount++ + } + if authMechanismsCount > 1 { + log.Warn(ctx, "API: Invalid login - multiple authentication mechanisms", "auth", "subsonic", "remoteAddr", r.RemoteAddr) + sendError(w, r, newError(responses.ErrorMultipleAuthMechanismsProvided)) + return + } + + if apiKey != "" { + usr, err = ds.User(ctx).FindByAPIKey(apiKey) + if errors.Is(err, context.Canceled) { + log.Debug(ctx, "API: Request canceled when authenticating", "auth", "subsonic-apikey", "remoteAddr", r.RemoteAddr, err) + return + } + if errors.Is(err, model.ErrNotFound) { + log.Warn(ctx, "API: Invalid login - API key not found", "auth", "subsonic-apikey", "remoteAddr", r.RemoteAddr) + } else if err != nil { + log.Error(ctx, "API: Error authenticating with API key", "auth", "subsonic-apikey", "remoteAddr", r.RemoteAddr, err) + } + } else { + usr, err = ds.User(ctx).FindByUsernameWithPassword(username) + if errors.Is(err, context.Canceled) { + log.Debug(ctx, "API: Request canceled when authenticating", "auth", "subsonic", "username", username, "remoteAddr", r.RemoteAddr, err) + return + } + switch { + case errors.Is(err, model.ErrNotFound): log.Warn(ctx, "API: Invalid login", "auth", "subsonic", "username", username, "remoteAddr", r.RemoteAddr, err) + case err != nil: + log.Error(ctx, "API: Error authenticating username", "auth", "subsonic", "username", username, "remoteAddr", r.RemoteAddr, err) + default: + err = validateCredentials(usr, pass, token, salt, jwt) + if err != nil { + log.Warn(ctx, "API: Invalid login", "auth", "subsonic", "username", username, "remoteAddr", r.RemoteAddr, err) + } } } } diff --git a/server/subsonic/middlewares_test.go b/server/subsonic/middlewares_test.go index a30d5b3af..e9f1c0177 100644 --- a/server/subsonic/middlewares_test.go +++ b/server/subsonic/middlewares_test.go @@ -306,6 +306,65 @@ var _ = Describe("Middlewares", func() { Expect(next.called).To(BeFalse()) }) }) + + When("using api key authentication", func() { + BeforeEach(func() { + DeferCleanup(configtest.SetupConfig()) + + ur := ds.User(context.TODO()) + user := &model.User{ + UserName: "user-api", + NewPassword: "wordpass-api", + } + _ = ur.Put(user) + + ar := ds.APIKey(context.TODO()) + apiKey := &model.APIKey{ + ID: "api-key-id", + UserID: user.ID, + Name: "API Key", + Key: "api-key", + } + _ = ar.Put(apiKey) + }) + + It("passes authentication with correct api key", func() { + r := newGetRequest("apiKey=api-key") + cp := authenticate(ds)(next) + cp.ServeHTTP(w, r) + + Expect(next.called).To(BeTrue()) + user, _ := request.UserFrom(next.req.Context()) + Expect(user.UserName).To(Equal("user-api")) + }) + + It("fails authentication with invalid api key", func() { + r := newGetRequest("apiKey=invalid-api-key") + cp := authenticate(ds)(next) + cp.ServeHTTP(w, r) + + Expect(w.Body.String()).To(ContainSubstring(`code="40"`)) + Expect(next.called).To(BeFalse()) + }) + + It("fails authentication with empty api key", func() { + r := newGetRequest("apiKey=") + cp := authenticate(ds)(next) + cp.ServeHTTP(w, r) + + Expect(w.Body.String()).To(ContainSubstring(`code="40"`)) + Expect(next.called).To(BeFalse()) + }) + + It("fails authentication if both api key and username are provided", func() { + r := newGetRequest("apiKey=api-key", "u=user-api") + cp := authenticate(ds)(next) + cp.ServeHTTP(w, r) + + Expect(w.Body.String()).To(ContainSubstring(`code="43"`)) + Expect(next.called).To(BeFalse()) + }) + }) }) Describe("GetPlayer", func() { diff --git a/server/subsonic/responses/errors.go b/server/subsonic/responses/errors.go index 42e5427b3..f869c763e 100644 --- a/server/subsonic/responses/errors.go +++ b/server/subsonic/responses/errors.go @@ -1,25 +1,31 @@ package responses const ( - ErrorGeneric int32 = 0 - ErrorMissingParameter int32 = 10 - ErrorClientTooOld int32 = 20 - ErrorServerTooOld int32 = 30 - ErrorAuthenticationFail int32 = 40 - ErrorAuthorizationFail int32 = 50 - ErrorTrialExpired int32 = 60 - ErrorDataNotFound int32 = 70 + ErrorGeneric int32 = 0 + ErrorMissingParameter int32 = 10 + ErrorClientTooOld int32 = 20 + ErrorServerTooOld int32 = 30 + ErrorAuthenticationFail int32 = 40 + ErrorTokenAuthNotSupported int32 = 41 + ErrorAuthMechanismNotSupported int32 = 42 + ErrorMultipleAuthMechanismsProvided int32 = 43 + ErrorAuthorizationFail int32 = 50 + ErrorTrialExpired int32 = 60 + ErrorDataNotFound int32 = 70 ) var errors = map[int32]string{ - ErrorGeneric: "A generic error", - ErrorMissingParameter: "Required parameter is missing", - ErrorClientTooOld: "Incompatible Subsonic REST protocol version. Client must upgrade", - ErrorServerTooOld: "Incompatible Subsonic REST protocol version. Server must upgrade", - ErrorAuthenticationFail: "Wrong username or password", - ErrorAuthorizationFail: "User is not authorized for the given operation", - ErrorTrialExpired: "The trial period for the Subsonic server is over. Please upgrade to Subsonic Premium. Visit subsonic.org for details", - ErrorDataNotFound: "The requested data was not found", + ErrorGeneric: "A generic error", + ErrorMissingParameter: "Required parameter is missing", + ErrorClientTooOld: "Incompatible Subsonic REST protocol version. Client must upgrade", + ErrorServerTooOld: "Incompatible Subsonic REST protocol version. Server must upgrade", + ErrorAuthenticationFail: "Wrong username or password or api key", + ErrorTokenAuthNotSupported: "Token authentication not supported", + ErrorAuthMechanismNotSupported: "Provided authentication mechanism not supported", + ErrorMultipleAuthMechanismsProvided: "Multiple conflicting authentication mechanisms provided", + ErrorAuthorizationFail: "User is not authorized for the given operation", + ErrorTrialExpired: "The trial period for the Subsonic server is over. Please upgrade to Subsonic Premium. Visit subsonic.org for details", + ErrorDataNotFound: "The requested data was not found", } func ErrorMsg(code int32) string { diff --git a/tests/mock_apikey_repo.go b/tests/mock_apikey_repo.go new file mode 100644 index 000000000..240281a13 --- /dev/null +++ b/tests/mock_apikey_repo.go @@ -0,0 +1,57 @@ +package tests + +import ( + "github.com/navidrome/navidrome/model" + "strings" +) + +func CreateMockApiKeyRepo() *MockedAPIKeyRepo { + return &MockedAPIKeyRepo{ + Data: map[string]*model.APIKey{}, + } +} + +type MockedAPIKeyRepo struct { + model.APIKeyRepository + Error error + Data map[string]*model.APIKey +} + +func (m *MockedAPIKeyRepo) CountAll(_ ...model.QueryOptions) (int64, error) { + if m.Error != nil { + return 0, m.Error + } + return int64(len(m.Data)), nil +} + +func (m *MockedAPIKeyRepo) Put(apiKey *model.APIKey) error { + if m.Error != nil { + return m.Error + } + m.Data[strings.ToLower(apiKey.Key)] = apiKey + return nil +} + +func (m *MockedAPIKeyRepo) FindByKey(key string) (*model.APIKey, error) { + if m.Error != nil { + return nil, m.Error + } + apiKey, exists := m.Data[strings.ToLower(key)] + if !exists { + return nil, model.ErrNotFound + } + return apiKey, nil +} + +func (m *MockedAPIKeyRepo) Get(id string) (*model.APIKey, error) { + if m.Error != nil { + return nil, m.Error + } + + for _, apiKey := range m.Data { + if apiKey.ID == id { + return apiKey, nil + } + } + return nil, model.ErrNotFound +} diff --git a/tests/mock_data_store.go b/tests/mock_data_store.go index 56f68a74b..93f2d8a68 100644 --- a/tests/mock_data_store.go +++ b/tests/mock_data_store.go @@ -28,6 +28,7 @@ type MockDataStore struct { MockedRadio model.RadioRepository scrobbleBufferMu sync.Mutex repoMu sync.Mutex + MockedAPIKey model.APIKeyRepository } func (db *MockDataStore) Library(ctx context.Context) model.LibraryRepository { @@ -169,7 +170,8 @@ func (db *MockDataStore) User(ctx context.Context) model.UserRepository { if db.RealDS != nil { db.MockedUser = db.RealDS.User(ctx) } else { - db.MockedUser = CreateMockUserRepo() + apiKeyRepo := db.APIKey(ctx).(*MockedAPIKeyRepo) + db.MockedUser = CreateMockUserRepo(apiKeyRepo) } } return db.MockedUser @@ -221,6 +223,17 @@ func (db *MockDataStore) Radio(ctx context.Context) model.RadioRepository { return db.MockedRadio } +func (db *MockDataStore) APIKey(ctx context.Context) model.APIKeyRepository { + if db.MockedAPIKey == nil { + if db.RealDS != nil { + db.MockedAPIKey = db.RealDS.APIKey(ctx) + } else { + db.MockedAPIKey = CreateMockApiKeyRepo() + } + } + return db.MockedAPIKey +} + func (db *MockDataStore) WithTx(block func(tx model.DataStore) error, label ...string) error { return block(db) } diff --git a/tests/mock_user_repo.go b/tests/mock_user_repo.go index 9f3dd672e..5cfd32cf1 100644 --- a/tests/mock_user_repo.go +++ b/tests/mock_user_repo.go @@ -10,10 +10,15 @@ import ( "github.com/navidrome/navidrome/utils/gg" ) -func CreateMockUserRepo() *MockedUserRepo { +func CreateMockUserRepo(apiKeyRepo ...*MockedAPIKeyRepo) *MockedUserRepo { + var repo *MockedAPIKeyRepo + if len(apiKeyRepo) > 0 { + repo = apiKeyRepo[0] + } return &MockedUserRepo{ Data: map[string]*model.User{}, UserLibraries: map[string][]int{}, + APIKeyRepo: repo, } } @@ -22,9 +27,10 @@ type MockedUserRepo struct { Error error Data map[string]*model.User UserLibraries map[string][]int // userID -> libraryIDs + APIKeyRepo *MockedAPIKeyRepo } -func (u *MockedUserRepo) CountAll(qo ...model.QueryOptions) (int64, error) { +func (u *MockedUserRepo) CountAll(_ ...model.QueryOptions) (int64, error) { if u.Error != nil { return 0, u.Error } @@ -123,3 +129,22 @@ func (u *MockedUserRepo) SetUserLibraries(userID string, libraryIDs []int) error u.UserLibraries[userID] = libraryIDs return nil } + +func (u *MockedUserRepo) FindByAPIKey(key string) (*model.User, error) { + if u.Error != nil { + return nil, u.Error + } + + apiKey, err := u.APIKeyRepo.FindByKey(key) + if err != nil { + return nil, err + } + + for _, usr := range u.Data { + if usr.ID == apiKey.UserID { + return usr, nil + } + } + + return nil, model.ErrNotFound +} diff --git a/ui/src/App.jsx b/ui/src/App.jsx index dc4fe9b53..7a5899045 100644 --- a/ui/src/App.jsx +++ b/ui/src/App.jsx @@ -42,6 +42,7 @@ import SharePlayer from './share/SharePlayer' import { HTML5Backend } from 'react-dnd-html5-backend' import { DndProvider } from 'react-dnd' import missing from './missing/index.js' +import apikey from './apikey/index.js' const history = createHashHistory() @@ -111,6 +112,11 @@ const Admin = (props) => { options={{ subMenu: 'playlist' }} />, , + , { + const translate = useTranslate() + const [mutate] = useMutation() + const notify = useNotify() + const redirect = useRedirect() + const resourceName = translate('resources.apikey.name', { smart_count: 1 }) + const title = translate('ra.page.create', { + name: `${resourceName}`, + }) + + const save = useCallback( + async (values) => { + try { + await mutate( + { + type: 'create', + resource: 'apikey', + payload: { data: values }, + }, + { returnPromise: true }, + ) + notify('resources.apikey.notifications.created', 'info', { + smart_count: 1, + }) + redirect('/apikey') + } catch (error) { + if (error.body.errors) { + return error.body.errors + } + } + }, + [mutate, notify, redirect], + ) + + return ( + } {...props}> + + + + + ) +} + +export default ApiKeyCreate diff --git a/ui/src/apikey/ApiKeyEdit.jsx b/ui/src/apikey/ApiKeyEdit.jsx new file mode 100644 index 000000000..cbcfa7396 --- /dev/null +++ b/ui/src/apikey/ApiKeyEdit.jsx @@ -0,0 +1,75 @@ +import React, { useCallback } from 'react' +import { + Edit, + SimpleForm, + TextInput, + required, + Toolbar, + SaveButton, + DeleteButton, + useTranslate, + useMutation, + useNotify, + useRefresh, + DateField, +} from 'react-admin' +import { Title } from '../common' + +const ApiKeyTitle = ({ record }) => { + const translate = useTranslate() + const resourceName = translate('resources.apikey.name', { smart_count: 1 }) + return +} + +const ApiKeyEditToolbar = (props) => ( + <Toolbar {...props}> + <SaveButton /> + <DeleteButton /> + </Toolbar> +) + +const ApiKeyEdit = (props) => { + const [mutate] = useMutation() + const notify = useNotify() + const refresh = useRefresh() + + const save = useCallback( + async (values) => { + try { + await mutate( + { + type: 'update', + resource: 'apikey', + payload: { id: values.id, data: values }, + }, + { returnPromise: true }, + ) + notify('resources.apikey.notifications.updated', 'info', { + smart_count: 1, + }) + refresh() + } catch (error) { + if (error.body.errors) { + return error.body.errors + } + } + }, + [mutate, notify, refresh], + ) + + return ( + <Edit title={<ApiKeyTitle />} {...props}> + <SimpleForm + toolbar={<ApiKeyEditToolbar />} + save={save} + variant={'outlined'} + > + <TextInput source="name" validate={[required()]} /> + <TextInput source="key" disabled fullWidth /> + <DateField variant="body1" source="createdAt" showTime /> + </SimpleForm> + </Edit> + ) +} + +export default ApiKeyEdit diff --git a/ui/src/apikey/ApiKeyList.jsx b/ui/src/apikey/ApiKeyList.jsx new file mode 100644 index 000000000..42a43aa15 --- /dev/null +++ b/ui/src/apikey/ApiKeyList.jsx @@ -0,0 +1,58 @@ +import React from 'react' +import { + Datagrid, + DateField, + Filter, + List, + SearchInput, + TextField, + useTranslate, +} from 'react-admin' +import { useMediaQuery } from '@material-ui/core' +import { SimpleList } from '../common' +import AddIcon from '@material-ui/icons/Add' +import { CreateButton } from 'react-admin' + +const ApiKeyFilter = (props) => ( + <Filter {...props} variant={'outlined'}> + <SearchInput id="search" source="name" alwaysOn /> + </Filter> +) + +const ApiKeyList = (props) => { + const isXsmall = useMediaQuery((theme) => theme.breakpoints.down('xs')) + const translate = useTranslate() + return ( + <List + {...props} + actions={ + <CreateButton + basePath="/apikey" + icon={<AddIcon />} + label={translate('resources.apikey.actions.add')} + /> + } + sort={{ field: 'createdAt', order: 'DESC' }} + exporter={false} + bulkActionButtons={false} + filters={<ApiKeyFilter />} + > + {isXsmall ? ( + <SimpleList + primaryText={(r) => r.name} + secondaryText={(r) => r.key} + tertiaryText={(r) => <DateField record={r} source="createdAt" />} + linkType={'edit'} + /> + ) : ( + <Datagrid rowClick="edit"> + <TextField source="name" /> + <TextField source="key" /> + <DateField source="createdAt" showTime /> + </Datagrid> + )} + </List> + ) +} + +export default ApiKeyList diff --git a/ui/src/apikey/index.js b/ui/src/apikey/index.js new file mode 100644 index 000000000..9c5c411c1 --- /dev/null +++ b/ui/src/apikey/index.js @@ -0,0 +1,11 @@ +import VpnKeyIcon from '@material-ui/icons/VpnKey' +import ApiKeyList from './ApiKeyList' +import ApiKeyCreate from './ApiKeyCreate' +import ApiKeyEdit from './ApiKeyEdit' + +export default { + list: ApiKeyList, + create: ApiKeyCreate, + edit: ApiKeyEdit, + icon: VpnKeyIcon, +} diff --git a/ui/src/i18n/en.json b/ui/src/i18n/en.json index 4a9039a67..48a8bc819 100644 --- a/ui/src/i18n/en.json +++ b/ui/src/i18n/en.json @@ -1,6 +1,22 @@ { "languageName": "English", "resources": { + "apikey": { + "name": "API Key |||| API Keys", + "fields": { + "name": "Name", + "key": "Key", + "createdAt": "Created At", + "key_help_text": "This key will be used for API authentication" + }, + "actions": { + "add": "Add API Key" + }, + "notifications": { + "created": "API Key created", + "updated": "API Key updated" + } + }, "song": { "name": "Song |||| Songs", "fields": { diff --git a/ui/src/layout/AppBar.jsx b/ui/src/layout/AppBar.jsx index 561701dce..9a8e7017b 100644 --- a/ui/src/layout/AppBar.jsx +++ b/ui/src/layout/AppBar.jsx @@ -63,6 +63,7 @@ AboutMenuItem.displayName = 'AboutMenuItem' const settingsResources = (resource) => resource.name !== 'user' && + resource.name !== 'apikey' && resource.hasList && resource.options && resource.options.subMenu === 'settings' @@ -95,6 +96,14 @@ const CustomUserMenu = ({ onClick, ...rest }) => { ) } + const renderApiKeyMenuItemLink = () => { + const apiKeyResource = resourceDefinition('apikey') + if (!apiKeyResource) { + return null + } + return renderSettingsMenuItemLink(apiKeyResource) + } + const renderSettingsMenuItemLink = (resource, id) => { const label = translate(`resources.${resource.name}.name`, { smart_count: id ? 1 : 2, @@ -128,6 +137,7 @@ const CustomUserMenu = ({ onClick, ...rest }) => { <PersonalMenu sidebarIsOpen={true} onClick={onClick} /> <Divider /> {renderUserMenuItemLink()} + {renderApiKeyMenuItemLink()} {resources .filter(settingsResources) .map((r) => renderSettingsMenuItemLink(r))} diff --git a/ui/src/layout/Menu.jsx b/ui/src/layout/Menu.jsx index 45f40b26d..cffbb3acf 100644 --- a/ui/src/layout/Menu.jsx +++ b/ui/src/layout/Menu.jsx @@ -103,7 +103,10 @@ const Menu = ({ dense = false }) => { } const subItems = (subMenu) => (resource) => - resource.hasList && resource.options && resource.options.subMenu === subMenu + resource.hasList && + resource.options && + resource.options.subMenu === subMenu && + resource.name !== 'apikey' return ( <div From b139f87c7eef19c731fddcb359fc8f907b12a789 Mon Sep 17 00:00:00 2001 From: Kartik Ohri <kartikohri13@gmail.com> Date: Wed, 21 May 2025 13:11:41 +0530 Subject: [PATCH 2/6] Apply suggestions from code review Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- server/subsonic/responses/errors.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/subsonic/responses/errors.go b/server/subsonic/responses/errors.go index f869c763e..b90f9260b 100644 --- a/server/subsonic/responses/errors.go +++ b/server/subsonic/responses/errors.go @@ -19,7 +19,7 @@ var errors = map[int32]string{ ErrorMissingParameter: "Required parameter is missing", ErrorClientTooOld: "Incompatible Subsonic REST protocol version. Client must upgrade", ErrorServerTooOld: "Incompatible Subsonic REST protocol version. Server must upgrade", - ErrorAuthenticationFail: "Wrong username or password or api key", + ErrorAuthenticationFail: "Invalid username, password, or API key", ErrorTokenAuthNotSupported: "Token authentication not supported", ErrorAuthMechanismNotSupported: "Provided authentication mechanism not supported", ErrorMultipleAuthMechanismsProvided: "Multiple conflicting authentication mechanisms provided", From 5756ec89ea4fae3e06fa154396fb7cde47392e64 Mon Sep 17 00:00:00 2001 From: Kartik Ohri <kartikohri13@gmail.com> Date: Wed, 21 May 2025 15:37:40 +0530 Subject: [PATCH 3/6] Add the toggle visibility button and copy to the clipboard button for API keys --- ui/src/apikey/ApiKeyList.jsx | 119 +++++++++++++++++++++++++++++++++-- 1 file changed, 114 insertions(+), 5 deletions(-) diff --git a/ui/src/apikey/ApiKeyList.jsx b/ui/src/apikey/ApiKeyList.jsx index 42a43aa15..2f656111f 100644 --- a/ui/src/apikey/ApiKeyList.jsx +++ b/ui/src/apikey/ApiKeyList.jsx @@ -1,17 +1,45 @@ -import React from 'react' +import React, { useState } from 'react' import { + CreateButton, Datagrid, DateField, Filter, + FunctionField, List, SearchInput, TextField, + useNotify, useTranslate, } from 'react-admin' -import { useMediaQuery } from '@material-ui/core' +import { + IconButton, + makeStyles, + Tooltip, + useMediaQuery, +} from '@material-ui/core' import { SimpleList } from '../common' import AddIcon from '@material-ui/icons/Add' -import { CreateButton } from 'react-admin' +import VisibilityIcon from '@material-ui/icons/Visibility' +import VisibilityOffIcon from '@material-ui/icons/VisibilityOff' +import FileCopyIcon from '@material-ui/icons/FileCopy' + +const useStyles = makeStyles({ + actionContainer: { + display: 'flex', + alignItems: 'center', + }, + keyContainer: { + display: 'flex', + alignItems: 'center', + flex: 1, + }, + visibilityButton: { + padding: 4, + }, + copyButton: { + padding: 4, + }, +}) const ApiKeyFilter = (props) => ( <Filter {...props} variant={'outlined'}> @@ -19,6 +47,80 @@ const ApiKeyFilter = (props) => ( </Filter> ) +const MaskedKeyField = ({ record, isSimpleMode }) => { + const [visible, setVisible] = useState(false) + const classes = useStyles() + const notify = useNotify() + + if (!record || !record.key) return null + + const maskKey = (key) => { + return '*'.repeat(key.length) + } + + const handleCopy = (e) => { + if (isSimpleMode) { + e.preventDefault() + } else { + e.stopPropagation() + } + navigator.clipboard.writeText(record.key) + notify('API key copied to clipboard', 'info') + } + + const toggleVisibility = (e) => { + if (isSimpleMode) { + e.preventDefault() + } else { + e.stopPropagation() + } + setVisible(!visible) + } + + const keyVisibilityButton = ( + <IconButton + className={classes.visibilityButton} + onClick={toggleVisibility} + size="small" + > + {visible ? ( + <VisibilityOffIcon fontSize="small" /> + ) : ( + <VisibilityIcon fontSize="small" /> + )} + </IconButton> + ) + const copyButton = ( + <IconButton + className={classes.copyButton} + onClick={handleCopy} + size="small" + > + <FileCopyIcon fontSize="small" /> + </IconButton> + ) + + return ( + <div className={classes.actionContainer}> + <div className={classes.keyContainer}> + {visible ? record.key : maskKey(record.key)} + </div> + {isSimpleMode ? ( + <>{keyVisibilityButton}</> + ) : ( + <Tooltip title={visible ? 'Hide key' : 'Show key'}> + {keyVisibilityButton} + </Tooltip> + )} + {isSimpleMode ? ( + <>{copyButton}</> + ) : ( + <Tooltip title="Copy to clipboard">{copyButton}</Tooltip> + )} + </div> + ) +} + const ApiKeyList = (props) => { const isXsmall = useMediaQuery((theme) => theme.breakpoints.down('xs')) const translate = useTranslate() @@ -40,14 +142,21 @@ const ApiKeyList = (props) => { {isXsmall ? ( <SimpleList primaryText={(r) => r.name} - secondaryText={(r) => r.key} + secondaryText={(r) => ( + <MaskedKeyField record={r} isSimpleMode={true} /> + )} tertiaryText={(r) => <DateField record={r} source="createdAt" />} linkType={'edit'} /> ) : ( <Datagrid rowClick="edit"> <TextField source="name" /> - <TextField source="key" /> + <FunctionField + label="Key" + render={(record) => ( + <MaskedKeyField isSimpleMode={false} record={record} /> + )} + /> <DateField source="createdAt" showTime /> </Datagrid> )} From c655a91a5d9b1cb9b755a257fd5f3326b5028fdc Mon Sep 17 00:00:00 2001 From: Kartik Ohri <kartikohri13@gmail.com> Date: Wed, 21 May 2025 15:54:15 +0530 Subject: [PATCH 4/6] Update tests/mock_user_repo.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- tests/mock_user_repo.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/mock_user_repo.go b/tests/mock_user_repo.go index 5cfd32cf1..5089dd18e 100644 --- a/tests/mock_user_repo.go +++ b/tests/mock_user_repo.go @@ -14,6 +14,8 @@ func CreateMockUserRepo(apiKeyRepo ...*MockedAPIKeyRepo) *MockedUserRepo { var repo *MockedAPIKeyRepo if len(apiKeyRepo) > 0 { repo = apiKeyRepo[0] + } else { + repo = CreateMockApiKeyRepo() } return &MockedUserRepo{ Data: map[string]*model.User{}, From d5b47383ae2f6e6acd93457f66bb436fe79beac1 Mon Sep 17 00:00:00 2001 From: Kartik Ohri <kartikohri13@gmail.com> Date: Sat, 26 Jul 2025 10:26:22 +0530 Subject: [PATCH 5/6] Associate API keys with players instead of users # Conflicts: # tests/mock_user_repo.go --- .../20250415111500_add_api_key_table.go | 8 +- model/api_key.go | 10 +- persistence/api_key_repository.go | 113 ++++++++---- persistence/api_key_repository_test.go | 167 ++++++++++-------- persistence/user_repository.go | 9 +- persistence/user_repository_test.go | 2 +- server/nativeapi/native_api.go | 37 ++++ server/subsonic/middlewares_test.go | 35 +++- tests/mock_apikey_repo.go | 9 +- tests/mock_data_store.go | 5 +- tests/mock_player_repo.go | 74 ++++++++ tests/mock_user_repo.go | 8 +- 12 files changed, 347 insertions(+), 130 deletions(-) create mode 100644 tests/mock_player_repo.go diff --git a/db/migrations/20250415111500_add_api_key_table.go b/db/migrations/20250415111500_add_api_key_table.go index d75ddd19f..a931d1827 100644 --- a/db/migrations/20250415111500_add_api_key_table.go +++ b/db/migrations/20250415111500_add_api_key_table.go @@ -15,18 +15,18 @@ func upAddApiKeyTable(ctx context.Context, tx *sql.Tx) error { _, err := tx.ExecContext(ctx, ` create table if not exists api_key ( id text not null primary key, - user_id text not null, + player_id text not null, name text not null, key text not null unique, created_at datetime not null, - foreign key (user_id) - references user(id) + foreign key (player_id) + references player(id) on delete cascade ); create index if not exists api_key_key on api_key(key); - create index if not exists api_key_user_id on api_key(user_id); + create index if not exists api_key_player_id on api_key(player_id); `) return err } diff --git a/model/api_key.go b/model/api_key.go index 6d238f422..63486c2fc 100644 --- a/model/api_key.go +++ b/model/api_key.go @@ -6,10 +6,10 @@ import ( ) type APIKey struct { - ID string `structs:"id" json:"id"` - UserID string `structs:"user_id" json:"userId"` - Name string `structs:"name" json:"name"` - Key string `structs:"key" json:"key"` + ID string `structs:"id" json:"id"` + PlayerID string `structs:"player_id" json:"playerId"` + Name string `structs:"name" json:"name"` + Key string `structs:"key" json:"key"` CreatedAt time.Time `structs:"created_at" json:"createdAt"` } @@ -21,6 +21,6 @@ type APIKeyRepository interface { CountAll(...QueryOptions) (int64, error) Get(id string) (*APIKey, error) GetAll(options ...QueryOptions) (APIKeys, error) - Put(*APIKey) error FindByKey(key string) (*APIKey, error) + RefreshKey(id string) (string, error) } diff --git a/persistence/api_key_repository.go b/persistence/api_key_repository.go index 583578d3c..be8e9d715 100644 --- a/persistence/api_key_repository.go +++ b/persistence/api_key_repository.go @@ -29,16 +29,16 @@ func (r *apiKeyRepository) userFilter() Sqlizer { if user.IsAdmin { return And{} } - return Eq{"user_id": user.ID} + return Eq{"p.user_id": user.ID} } func (r *apiKeyRepository) CountAll(options ...model.QueryOptions) (int64, error) { - sq := Select().From(r.tableName).Where(r.userFilter()) + sq := r.selectAPIKey(options...).Where(r.userFilter()) return r.count(sq, options...) } func (r *apiKeyRepository) Get(id string) (*model.APIKey, error) { - sel := r.newSelect().Columns("*").Where(And{Eq{"id": id}}) + sel := r.selectAPIKey().Where(And{Eq{"ak.id": id}}) var res model.APIKey err := r.queryOne(sel, &res) if err != nil { @@ -47,8 +47,15 @@ func (r *apiKeyRepository) Get(id string) (*model.APIKey, error) { return &res, err } +func (r *apiKeyRepository) selectAPIKey(options ...model.QueryOptions) SelectBuilder { + return r.newSelect(options...). + From("api_key ak"). + LeftJoin("player p ON ak.player_id = p.id"). + Columns("ak.*") +} + func (r *apiKeyRepository) GetAll(options ...model.QueryOptions) (model.APIKeys, error) { - sel := r.newSelect(options...).Columns("*").Where(r.userFilter()) + sel := r.selectAPIKey().Where(r.userFilter()) res := model.APIKeys{} err := r.queryAll(sel, &res) if err != nil { @@ -57,32 +64,17 @@ func (r *apiKeyRepository) GetAll(options ...model.QueryOptions) (model.APIKeys, return res, err } -func (r *apiKeyRepository) Put(ak *model.APIKey) error { - if ak.ID == "" { - ak.ID = id.NewRandom() - } - ak.CreatedAt = time.Now() - values, err := toSQLArgs(*ak) - if err != nil { - return err - } - insert := Insert(r.tableName).SetMap(values) - _, err = r.executeSQL(insert) - return err -} - func (r *apiKeyRepository) Count(options ...rest.QueryOptions) (int64, error) { return r.CountAll(r.parseRestOptions(r.ctx, options...)) } func (r *apiKeyRepository) Read(id string) (interface{}, error) { - user := loggedUser(r.ctx) apiKey, err := r.Get(id) if err != nil { return nil, err } - if !user.IsAdmin && apiKey.UserID != user.ID { - return nil, rest.ErrPermissionDenied + if err := r.VerifyPlayerAccess(apiKey.PlayerID); err != nil { + return nil, err } return apiKey, err } @@ -101,14 +93,22 @@ func (r *apiKeyRepository) NewInstance() interface{} { func (r *apiKeyRepository) Save(entity interface{}) (string, error) { ak := entity.(*model.APIKey) - user := loggedUser(r.ctx) - ak.UserID = user.ID - // prefix API keys with nav_ - ak.Key = "nav_" + id.NewRandom() - err := r.Put(ak) + if err := r.VerifyPlayerAccess(ak.PlayerID); err != nil { + return "", err + } + + if ak.ID == "" { + ak.ID = id.NewRandom() + } + ak.Key = generateAPIKey() + ak.CreatedAt = time.Now() + values, err := toSQLArgs(*ak) if err != nil { return "", err } + + insert := Insert(r.tableName).SetMap(values) + _, err = r.executeSQL(insert) return ak.ID, err } @@ -118,12 +118,11 @@ func (r *apiKeyRepository) Update(id string, entity interface{}, _ ...string) er if err != nil { return err } - user := loggedUser(r.ctx) - if !user.IsAdmin && current.UserID != user.ID { - return rest.ErrPermissionDenied + + if err := r.VerifyPlayerAccess(current.PlayerID); err != nil { + return err } - // Only allow updating name update := Update(r.tableName). Set("name", ak.Name). Where(Eq{"id": id}) @@ -132,19 +131,20 @@ func (r *apiKeyRepository) Update(id string, entity interface{}, _ ...string) er } func (r *apiKeyRepository) Delete(id string) error { - user := loggedUser(r.ctx) apiKey, err := r.Get(id) if err != nil { return err } - if !user.IsAdmin && apiKey.UserID != user.ID { - return rest.ErrPermissionDenied + + if err := r.VerifyPlayerAccess(apiKey.PlayerID); err != nil { + return err } + return r.delete(Eq{"id": id}) } func (r *apiKeyRepository) FindByKey(key string) (*model.APIKey, error) { - sel := r.newSelect().Columns("*").Where(Eq{"key": key}) + sel := r.selectAPIKey().Where(And{Eq{"ak.key": key}}) var res model.APIKey err := r.queryOne(sel, &res) if err != nil { @@ -153,6 +153,51 @@ func (r *apiKeyRepository) FindByKey(key string) (*model.APIKey, error) { return &res, err } +func (r *apiKeyRepository) RefreshKey(id string) (string, error) { + apiKey, err := r.Get(id) + if err != nil { + return "", err + } + if err := r.VerifyPlayerAccess(apiKey.PlayerID); err != nil { + return "", err + } + + newKey := generateAPIKey() + update := Update(r.tableName). + Set("key", newKey). + Where(Eq{"id": id}) + _, err = r.executeSQL(update) + + if err != nil { + return "", err + } + + return newKey, nil +} + +func (r *apiKeyRepository) VerifyPlayerAccess(playerID string) error { + if playerID == "" { + return model.ErrNotFound + } + + playerRepo := NewPlayerRepository(r.ctx, r.db) + player, err := playerRepo.Get(playerID) + if err != nil { + return err + } + + user := loggedUser(r.ctx) + if !user.IsAdmin && player.UserId != user.ID { + return rest.ErrPermissionDenied + } + + return nil +} + +func generateAPIKey() string { + return "nav_" + id.NewRandom() +} + var _ model.APIKeyRepository = (*apiKeyRepository)(nil) var _ rest.Repository = (*apiKeyRepository)(nil) var _ rest.Persistable = (*apiKeyRepository)(nil) diff --git a/persistence/api_key_repository_test.go b/persistence/api_key_repository_test.go index 44ab6f0cc..d7df1504b 100644 --- a/persistence/api_key_repository_test.go +++ b/persistence/api_key_repository_test.go @@ -8,64 +8,49 @@ import ( "github.com/navidrome/navidrome/model/request" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/pocketbase/dbx" ) var _ = Describe("APIKeyRepository", func() { var repo model.APIKeyRepository + var playerRepo model.PlayerRepository + var database *dbx.DB + + var ( + adminPlayer = model.Player{ID: "1", Name: "NavidromeUI [Firefox/Linux]", UserAgent: "Firefox/Linux", UserId: adminUser.ID, Username: adminUser.UserName, Client: "NavidromeUI", IP: "127.0.0.1", ReportRealPath: true, ScrobbleEnabled: true} + regularPlayer = model.Player{ID: "3", Name: "NavidromeUI [Safari/macOS]", UserAgent: "Safari/macOS", UserId: regularUser.ID, Username: regularUser.UserName, Client: "NavidromeUI", ReportRealPath: true, ScrobbleEnabled: false} + + players = model.Players{adminPlayer, regularPlayer} + ) BeforeEach(func() { ctx := log.NewContext(context.TODO()) - ctx = request.WithUser(ctx, model.User{ID: "userid", UserName: "userid", IsAdmin: true}) - repo = NewAPIKeyRepository(ctx, GetDBXBuilder()) - }) + ctx = request.WithUser(ctx, adminUser) + database = GetDBXBuilder() - Describe("Put", func() { - It("sets an ID if it is not set", func() { - apiKey := &model.APIKey{ - UserID: "userid", - Name: "Test API Key", - Key: "test-key", - } - - err := repo.Put(apiKey) - - Expect(err).ToNot(HaveOccurred()) - Expect(apiKey.ID).ToNot(BeEmpty()) - Expect(apiKey.CreatedAt).ToNot(BeZero()) - }) - - It("keeps existing values", func() { - apiKey := &model.APIKey{ - ID: "existing-id", - UserID: "userid", - Name: "Test API Key 2", - Key: "test-key-2", - } - - err := repo.Put(apiKey) - - Expect(err).ToNot(HaveOccurred()) - Expect(apiKey.ID).To(Equal("existing-id")) - Expect(apiKey.CreatedAt).ToNot(BeZero()) - }) + playerRepo = NewPlayerRepository(ctx, database) + for idx := range players { + err := playerRepo.Put(&players[idx]) + Expect(err).To(BeNil()) + } + repo = NewAPIKeyRepository(ctx, database) }) Describe("FindByKey", func() { It("returns the API key with matching key", func() { apiKey := &model.APIKey{ - UserID: "userid", - Name: "Unique API Key", - Key: "unique-test-key", + PlayerID: adminPlayer.ID, + Name: "Unique API Key", } - - err := repo.Put(apiKey) + apiKeyId, err := repo.Save(apiKey) + Expect(err).ToNot(HaveOccurred()) + apiKey, err = repo.Get(apiKeyId) Expect(err).ToNot(HaveOccurred()) - result, err := repo.FindByKey("unique-test-key") - + result, err := repo.FindByKey(apiKey.Key) Expect(err).ToNot(HaveOccurred()) Expect(result.ID).To(Equal(apiKey.ID)) - Expect(result.Key).To(Equal("unique-test-key")) + Expect(result.Key).To(Equal(apiKey.Key)) }) It("returns error when key not found", func() { @@ -77,7 +62,8 @@ var _ = Describe("APIKeyRepository", func() { Describe("Save", func() { It("creates a new API key with a generated key", func() { apiKey := &model.APIKey{ - Name: "Test API Key Save", + Name: "Test API Key Save", + PlayerID: adminPlayer.ID, } id, err := repo.Save(apiKey) @@ -85,25 +71,24 @@ var _ = Describe("APIKeyRepository", func() { Expect(err).ToNot(HaveOccurred()) Expect(id).ToNot(BeEmpty()) Expect(apiKey.Key).To(HavePrefix("nav_")) - Expect(apiKey.UserID).To(Equal("userid")) + Expect(apiKey.PlayerID).To(Equal(adminPlayer.ID)) + Expect(apiKey.Name).To(Equal("Test API Key Save")) }) }) Describe("Update", func() { It("only updates the name field", func() { apiKey := &model.APIKey{ - UserID: "userid", - Name: "Original Name", - Key: "test-key-for-update", + PlayerID: adminPlayer.ID, + Name: "Original Name", } - err := repo.Put(apiKey) + _, err := repo.Save(apiKey) Expect(err).ToNot(HaveOccurred()) updateKey := &model.APIKey{ - Name: "Updated Name", - Key: "should-not-change", - UserID: "2222", + Name: "Updated Name", + PlayerID: regularPlayer.ID, } err = repo.Update(apiKey.ID, updateKey) @@ -112,8 +97,8 @@ var _ = Describe("APIKeyRepository", func() { result, err := repo.Get(apiKey.ID) Expect(err).ToNot(HaveOccurred()) Expect(result.Name).To(Equal("Updated Name")) - Expect(result.Key).To(Equal("test-key-for-update")) - Expect(result.UserID).To(Equal("userid")) + Expect(result.Key).To(Equal(apiKey.Key)) + Expect(result.PlayerID).To(Equal(adminPlayer.ID)) }) It("returns error when attempting to update non-existent key", func() { @@ -125,12 +110,11 @@ var _ = Describe("APIKeyRepository", func() { Describe("Delete", func() { It("deletes an existing API key", func() { apiKey := &model.APIKey{ - UserID: "userid", - Name: "API Key to Delete", - Key: "key-to-delete", + PlayerID: adminPlayer.ID, + Name: "API Key to Delete", } - err := repo.Put(apiKey) + _, err := repo.Save(apiKey) Expect(err).ToNot(HaveOccurred()) err = repo.Delete(apiKey.ID) @@ -141,6 +125,53 @@ var _ = Describe("APIKeyRepository", func() { }) }) + Describe("RefreshKey", func() { + It("generates a new key for an existing API key", func() { + apiKey := &model.APIKey{ + PlayerID: adminPlayer.ID, + Name: "Test Refresh", + } + _, err := repo.Save(apiKey) + Expect(err).ToNot(HaveOccurred()) + + originalKey := apiKey.Key + + newKey, err := repo.RefreshKey(apiKey.ID) + + Expect(err).ToNot(HaveOccurred()) + Expect(newKey).ToNot(BeEmpty()) + Expect(newKey).ToNot(Equal(originalKey)) + Expect(newKey).To(HavePrefix("nav_")) + + refreshed, err := repo.Get(apiKey.ID) + Expect(err).ToNot(HaveOccurred()) + Expect(refreshed.Key).To(Equal(newKey)) + }) + + It("returns an error for non-existent API key", func() { + _, err := repo.RefreshKey("non-existent-id") + Expect(err).To(HaveOccurred()) + Expect(err).To(MatchError(rest.ErrNotFound)) + }) + + It("enforces user permissions", func() { + apiKey := &model.APIKey{ + PlayerID: adminPlayer.ID, + Name: "Test Permission", + } + _, err := repo.Save(apiKey) + Expect(err).ToNot(HaveOccurred()) + + nonAdminCtx := log.NewContext(context.TODO()) + nonAdminCtx = request.WithUser(nonAdminCtx, regularUser) + nonAdminRepo := NewAPIKeyRepository(nonAdminCtx, database) + + _, err = nonAdminRepo.RefreshKey(apiKey.ID) + Expect(err).To(HaveOccurred()) + Expect(err).To(MatchError(rest.ErrPermissionDenied)) + }) + }) + Describe("User permissions", func() { var nonAdminCtx context.Context var nonAdminRepo model.APIKeyRepository @@ -148,8 +179,8 @@ var _ = Describe("APIKeyRepository", func() { BeforeEach(func() { nonAdminCtx = log.NewContext(context.TODO()) - nonAdminCtx = context.WithValue(nonAdminCtx, "user", model.User{ID: "2222", UserName: "user", IsAdmin: false}) - nonAdminRepo = NewAPIKeyRepository(nonAdminCtx, GetDBXBuilder()) + nonAdminCtx = request.WithUser(nonAdminCtx, regularUser) + nonAdminRepo = NewAPIKeyRepository(nonAdminCtx, database) cleanupKeys := func(key string) { foundKey, err := repo.FindByKey(key) @@ -161,20 +192,18 @@ var _ = Describe("APIKeyRepository", func() { cleanupKeys("user-key") tmpAdminKey := &model.APIKey{ - UserID: "userid", - Name: "Admin's API Key", - Key: "admin-key", + PlayerID: adminPlayer.ID, + Name: "Admin's API Key", } - err := repo.Put(tmpAdminKey) + _, err := repo.Save(tmpAdminKey) Expect(err).ToNot(HaveOccurred()) adminKey = *tmpAdminKey userKey := &model.APIKey{ - UserID: "2222", - Name: "User's API Key", - Key: "user-key", + PlayerID: regularPlayer.ID, + Name: "User's API Key", } - err = repo.Put(userKey) + _, err = repo.Save(userKey) Expect(err).ToNot(HaveOccurred()) }) @@ -183,7 +212,7 @@ var _ = Describe("APIKeyRepository", func() { Expect(err).ToNot(HaveOccurred()) for _, key := range results { - Expect(key.UserID).To(Equal("2222")) + Expect(key.PlayerID).To(Equal(regularPlayer.ID)) } }) @@ -193,11 +222,11 @@ var _ = Describe("APIKeyRepository", func() { userIds := make(map[string]bool) for _, key := range results { - userIds[key.UserID] = true + userIds[key.PlayerID] = true } - Expect(userIds).To(HaveKey("userid")) - Expect(userIds).To(HaveKey("2222")) + Expect(userIds).To(HaveKey(adminPlayer.ID)) + Expect(userIds).To(HaveKey(regularPlayer.ID)) }) It("a user cannot view/delete/update another user's key", func() { diff --git a/persistence/user_repository.go b/persistence/user_repository.go index c675228d3..ee057e373 100644 --- a/persistence/user_repository.go +++ b/persistence/user_repository.go @@ -195,14 +195,17 @@ func (r *userRepository) FindByUsernameWithPassword(username string) (*model.Use func (r *userRepository) FindByAPIKey(key string) (*model.User, error) { // find the API key in the database + playerRepo := NewPlayerRepository(r.ctx, r.db) apiKeyRepo := NewAPIKeyRepository(r.ctx, r.db) apiKey, err := apiKeyRepo.FindByKey(key) if err != nil { return nil, err } - - // Then get the user associated with this API key - return r.Get(apiKey.UserID) + player, err := playerRepo.Get(apiKey.PlayerID) + if err != nil { + return nil, err + } + return r.Get(player.UserId) } func (r *userRepository) UpdateLastLoginAt(id string) error { diff --git a/persistence/user_repository_test.go b/persistence/user_repository_test.go index 7c0707ecd..615c336f5 100644 --- a/persistence/user_repository_test.go +++ b/persistence/user_repository_test.go @@ -212,7 +212,7 @@ var _ = Describe("UserRepository", func() { var existingUser *model.User BeforeEach(func() { existingUser = &model.User{ID: "1", UserName: "johndoe"} - repo = tests.CreateMockUserRepo() + repo = tests.CreateMockUserRepo(nil, nil) err := repo.Put(existingUser) Expect(err).ToNot(HaveOccurred()) }) diff --git a/server/nativeapi/native_api.go b/server/nativeapi/native_api.go index c6d6e9c64..cab45fd6a 100644 --- a/server/nativeapi/native_api.go +++ b/server/nativeapi/native_api.go @@ -3,6 +3,8 @@ package nativeapi import ( "context" "encoding/json" + "fmt" + "github.com/navidrome/navidrome/utils/req" "html" "net/http" "strconv" @@ -74,6 +76,7 @@ func (n *Router) routes() http.Handler { n.addUserLibraryRoute(r) n.RX(r, "/library", n.libs.NewRepository, true) }) + n.addRefreshApiKeyRoute(r) }) return r @@ -247,3 +250,37 @@ func adminOnlyMiddleware(next http.Handler) http.Handler { next.ServeHTTP(w, r) }) } + +func (n *Router) addRefreshApiKeyRoute(r chi.Router) { + r.With(server.URLParamsMiddleware).Post("/apikey/{id}/refresh", func(w http.ResponseWriter, r *http.Request) { + p := req.Params(r) + id, err := p.String(":id") + if err != nil { + msg := fmt.Sprintf("api key id could not be parsed: %s", id) + log.Warn(msg) + http.Error(w, "not found", http.StatusNotFound) + return + } + + repo := n.ds.APIKey(r.Context()) + _, err = repo.RefreshKey(id) + if err != nil { + log.Error(r.Context(), "error refreshing api key", "id", id, err) + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + + updatedKey, err := repo.Get(id) + if err != nil { + log.Error(r.Context(), "error retrieving refreshed api key", "id", id, err) + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + err = rest.RespondWithJSON(w, http.StatusOK, updatedKey) + if err != nil { + log.Error(r.Context(), "error marshaling refreshed api key", "id", id, err) + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + }) +} diff --git a/server/subsonic/middlewares_test.go b/server/subsonic/middlewares_test.go index e9f1c0177..ee3879c09 100644 --- a/server/subsonic/middlewares_test.go +++ b/server/subsonic/middlewares_test.go @@ -308,6 +308,8 @@ var _ = Describe("Middlewares", func() { }) When("using api key authentication", func() { + var apiKey *model.APIKey + BeforeEach(func() { DeferCleanup(configtest.SetupConfig()) @@ -318,18 +320,35 @@ var _ = Describe("Middlewares", func() { } _ = ur.Put(user) - ar := ds.APIKey(context.TODO()) - apiKey := &model.APIKey{ - ID: "api-key-id", - UserID: user.ID, - Name: "API Key", - Key: "api-key", + pr := ds.Player(context.TODO()) + player := &model.Player{ + ID: "player1", + Name: "Test Player", + UserAgent: "Test/1.0", + UserId: user.ID, + Client: "test-client", + IP: "127.0.0.1", + LastSeen: time.Now(), + TranscodingId: "", + MaxBitRate: 320, + ReportRealPath: false, + ScrobbleEnabled: true, } - _ = ar.Put(apiKey) + _ = pr.Put(player) + + ar := ds.APIKey(context.TODO()) + newApiKey := &model.APIKey{ + ID: "api-key-id", + Name: "API Key", + PlayerID: player.ID, + } + apiKeyId, _ := ar.Save(newApiKey) + newApiKey, _ = ar.Get(apiKeyId) + apiKey = newApiKey }) It("passes authentication with correct api key", func() { - r := newGetRequest("apiKey=api-key") + r := newGetRequest("apiKey=" + apiKey.Key) cp := authenticate(ds)(next) cp.ServeHTTP(w, r) diff --git a/tests/mock_apikey_repo.go b/tests/mock_apikey_repo.go index 240281a13..b99f2316c 100644 --- a/tests/mock_apikey_repo.go +++ b/tests/mock_apikey_repo.go @@ -2,6 +2,7 @@ package tests import ( "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/model/id" "strings" ) @@ -24,12 +25,14 @@ func (m *MockedAPIKeyRepo) CountAll(_ ...model.QueryOptions) (int64, error) { return int64(len(m.Data)), nil } -func (m *MockedAPIKeyRepo) Put(apiKey *model.APIKey) error { +func (m *MockedAPIKeyRepo) Save(entity interface{}) (string, error) { if m.Error != nil { - return m.Error + return "", m.Error } + apiKey := entity.(*model.APIKey) + apiKey.Key = "nav_" + id.NewRandom() m.Data[strings.ToLower(apiKey.Key)] = apiKey - return nil + return apiKey.ID, nil } func (m *MockedAPIKeyRepo) FindByKey(key string) (*model.APIKey, error) { diff --git a/tests/mock_data_store.go b/tests/mock_data_store.go index 93f2d8a68..5945c7faa 100644 --- a/tests/mock_data_store.go +++ b/tests/mock_data_store.go @@ -170,8 +170,9 @@ func (db *MockDataStore) User(ctx context.Context) model.UserRepository { if db.RealDS != nil { db.MockedUser = db.RealDS.User(ctx) } else { + playerRepo := db.Player(ctx).(*MockedPlayerRepo) apiKeyRepo := db.APIKey(ctx).(*MockedAPIKeyRepo) - db.MockedUser = CreateMockUserRepo(apiKeyRepo) + db.MockedUser = CreateMockUserRepo(playerRepo, apiKeyRepo) } } return db.MockedUser @@ -193,7 +194,7 @@ func (db *MockDataStore) Player(ctx context.Context) model.PlayerRepository { if db.RealDS != nil { db.MockedPlayer = db.RealDS.Player(ctx) } else { - db.MockedPlayer = struct{ model.PlayerRepository }{} + db.MockedPlayer = CreateMockPlayerRepo() } } return db.MockedPlayer diff --git a/tests/mock_player_repo.go b/tests/mock_player_repo.go new file mode 100644 index 000000000..b84cc2663 --- /dev/null +++ b/tests/mock_player_repo.go @@ -0,0 +1,74 @@ +package tests + +import ( + "encoding/base64" + "github.com/navidrome/navidrome/model" +) + +func CreateMockPlayerRepo() *MockedPlayerRepo { + return &MockedPlayerRepo{ + Data: make(map[string]*model.Player), + } +} + +type MockedPlayerRepo struct { + model.PlayerRepository + Error error + Data map[string]*model.Player +} + +func (m *MockedPlayerRepo) Get(id string) (*model.Player, error) { + if m.Error != nil { + return nil, m.Error + } + + player, exists := m.Data[id] + if !exists { + return nil, model.ErrNotFound + } + return player, nil +} + +func (m *MockedPlayerRepo) FindMatch(userId, client, userAgent string) (*model.Player, error) { + if m.Error != nil { + return nil, m.Error + } + + for _, player := range m.Data { + if player.UserId == userId && player.Client == client && player.UserAgent == userAgent { + return player, nil + } + } + return nil, model.ErrNotFound +} + +func (m *MockedPlayerRepo) Put(p *model.Player) error { + if m.Error != nil { + return m.Error + } + + if p.ID == "" { + p.ID = base64.StdEncoding.EncodeToString([]byte(p.Name + "_" + p.UserId + "_" + p.Client)) + } + m.Data[p.ID] = p + return nil +} + +func (m *MockedPlayerRepo) CountAll(_ ...model.QueryOptions) (int64, error) { + if m.Error != nil { + return 0, m.Error + } + return int64(len(m.Data)), nil +} + +func (m *MockedPlayerRepo) CountByClient(_ ...model.QueryOptions) (map[string]int64, error) { + if m.Error != nil { + return nil, m.Error + } + + result := make(map[string]int64) + for _, player := range m.Data { + result[player.Client]++ + } + return result, nil +} diff --git a/tests/mock_user_repo.go b/tests/mock_user_repo.go index 5089dd18e..bc88fc04c 100644 --- a/tests/mock_user_repo.go +++ b/tests/mock_user_repo.go @@ -30,6 +30,7 @@ type MockedUserRepo struct { Data map[string]*model.User UserLibraries map[string][]int // userID -> libraryIDs APIKeyRepo *MockedAPIKeyRepo + PlayerRepo *MockedPlayerRepo } func (u *MockedUserRepo) CountAll(_ ...model.QueryOptions) (int64, error) { @@ -142,8 +143,13 @@ func (u *MockedUserRepo) FindByAPIKey(key string) (*model.User, error) { return nil, err } + player, err := u.PlayerRepo.Get(apiKey.PlayerID) + if err != nil { + return nil, err + } + for _, usr := range u.Data { - if usr.ID == apiKey.UserID { + if usr.ID == player.UserId { return usr, nil } } From aecf959d98e968ad5c3c036b72c8a6d0241b4bd3 Mon Sep 17 00:00:00 2001 From: Kartik Ohri <kartikohri13@gmail.com> Date: Sun, 10 Aug 2025 02:21:43 +0530 Subject: [PATCH 6/6] Move api key to player model --- .../20250415111500_add_api_key_table.go | 39 --- ...50730104020_add_api_key_to_player_table.go | 34 +++ model/api_key.go | 26 -- model/datastore.go | 1 - model/player.go | 3 + model/user.go | 2 - persistence/api_key_repository.go | 203 --------------- persistence/api_key_repository_test.go | 245 ------------------ persistence/persistence.go | 6 - persistence/player_repository.go | 63 ++++- persistence/player_repository_test.go | 97 ++++--- persistence/user_repository.go | 15 -- persistence/user_repository_test.go | 2 +- server/nativeapi/native_api.go | 82 +++--- server/subsonic/middlewares.go | 20 +- server/subsonic/middlewares_test.go | 14 +- tests/mock_apikey_repo.go | 60 ----- tests/mock_data_store.go | 16 +- tests/mock_player_repo.go | 14 + tests/mock_user_repo.go | 35 +-- ui/src/App.jsx | 6 - ui/src/i18n/en.json | 22 +- ui/src/player/PlayerCreate.jsx | 44 ++++ ui/src/player/PlayerEdit.jsx | 182 +++++++++++-- ui/src/player/index.js | 2 + 25 files changed, 441 insertions(+), 792 deletions(-) delete mode 100644 db/migrations/20250415111500_add_api_key_table.go create mode 100644 db/migrations/20250730104020_add_api_key_to_player_table.go delete mode 100644 model/api_key.go delete mode 100644 persistence/api_key_repository.go delete mode 100644 persistence/api_key_repository_test.go delete mode 100644 tests/mock_apikey_repo.go create mode 100644 ui/src/player/PlayerCreate.jsx diff --git a/db/migrations/20250415111500_add_api_key_table.go b/db/migrations/20250415111500_add_api_key_table.go deleted file mode 100644 index a931d1827..000000000 --- a/db/migrations/20250415111500_add_api_key_table.go +++ /dev/null @@ -1,39 +0,0 @@ -package migrations - -import ( - "context" - "database/sql" - - "github.com/pressly/goose/v3" -) - -func init() { - goose.AddMigrationContext(upAddApiKeyTable, downAddApiKeyTable) -} - -func upAddApiKeyTable(ctx context.Context, tx *sql.Tx) error { - _, err := tx.ExecContext(ctx, ` - create table if not exists api_key ( - id text not null primary key, - player_id text not null, - name text not null, - key text not null unique, - created_at datetime not null, - - foreign key (player_id) - references player(id) - on delete cascade - ); - - create index if not exists api_key_key on api_key(key); - create index if not exists api_key_player_id on api_key(player_id); -`) - return err -} - -func downAddApiKeyTable(ctx context.Context, tx *sql.Tx) error { - _, err := tx.ExecContext(ctx, ` - drop table api_key; -`) - return err -} diff --git a/db/migrations/20250730104020_add_api_key_to_player_table.go b/db/migrations/20250730104020_add_api_key_to_player_table.go new file mode 100644 index 000000000..34b887d54 --- /dev/null +++ b/db/migrations/20250730104020_add_api_key_to_player_table.go @@ -0,0 +1,34 @@ +package migrations + +import ( + "context" + "database/sql" + + "github.com/pressly/goose/v3" +) + +func init() { + goose.AddMigrationContext(upAddAPIKeyToPlayer, downDropAPIKeyFromPlayer) +} + +func upAddAPIKeyToPlayer(_ context.Context, tx *sql.Tx) error { + _, err := tx.Exec(` +-- Add nullable api_key column to player table +ALTER TABLE player ADD COLUMN api_key VARCHAR(255); + +-- Add index on api_key for faster lookups +CREATE INDEX IF NOT EXISTS player_api_key ON player(api_key); +`) + return err +} + +func downDropAPIKeyFromPlayer(_ context.Context, tx *sql.Tx) error { + _, err := tx.Exec(` +-- Drop the index first +DROP INDEX IF EXISTS player_api_key; + +-- Then drop the column +ALTER TABLE player DROP COLUMN api_key; +`) + return err +} diff --git a/model/api_key.go b/model/api_key.go deleted file mode 100644 index 63486c2fc..000000000 --- a/model/api_key.go +++ /dev/null @@ -1,26 +0,0 @@ -package model - -import ( - "github.com/deluan/rest" - "time" -) - -type APIKey struct { - ID string `structs:"id" json:"id"` - PlayerID string `structs:"player_id" json:"playerId"` - Name string `structs:"name" json:"name"` - Key string `structs:"key" json:"key"` - CreatedAt time.Time `structs:"created_at" json:"createdAt"` -} - -type APIKeys []APIKey - -type APIKeyRepository interface { - ResourceRepository - rest.Persistable - CountAll(...QueryOptions) (int64, error) - Get(id string) (*APIKey, error) - GetAll(options ...QueryOptions) (APIKeys, error) - FindByKey(key string) (*APIKey, error) - RefreshKey(id string) (string, error) -} diff --git a/model/datastore.go b/model/datastore.go index ebd99e25b..4290e2134 100644 --- a/model/datastore.go +++ b/model/datastore.go @@ -38,7 +38,6 @@ type DataStore interface { User(ctx context.Context) UserRepository UserProps(ctx context.Context) UserPropsRepository ScrobbleBuffer(ctx context.Context) ScrobbleBufferRepository - APIKey(ctx context.Context) APIKeyRepository Resource(ctx context.Context, model interface{}) ResourceRepository diff --git a/model/player.go b/model/player.go index 39ea99d1a..f896ebd25 100644 --- a/model/player.go +++ b/model/player.go @@ -18,6 +18,7 @@ type Player struct { MaxBitRate int `structs:"max_bit_rate" json:"maxBitRate"` ReportRealPath bool `structs:"report_real_path" json:"reportRealPath"` ScrobbleEnabled bool `structs:"scrobble_enabled" json:"scrobbleEnabled"` + APIKey string `structs:"api_key" json:"apiKey" db:"api_key"` } type Players []Player @@ -28,4 +29,6 @@ type PlayerRepository interface { Put(p *Player) error CountAll(...QueryOptions) (int64, error) CountByClient(...QueryOptions) (map[string]int64, error) + FindByAPIKey(key string) (*Player, error) + GenerateAPIKey(playerId string) (string, error) } diff --git a/model/user.go b/model/user.go index 042ac8d60..aabedc096 100644 --- a/model/user.go +++ b/model/user.go @@ -56,6 +56,4 @@ type UserRepository interface { // Library association methods GetUserLibraries(userID string) (Libraries, error) SetUserLibraries(userID string, libraryIDs []int) error - // FindByAPIKey finds a user by the provided API key - FindByAPIKey(key string) (*User, error) } diff --git a/persistence/api_key_repository.go b/persistence/api_key_repository.go deleted file mode 100644 index be8e9d715..000000000 --- a/persistence/api_key_repository.go +++ /dev/null @@ -1,203 +0,0 @@ -package persistence - -import ( - "context" - "time" - - "github.com/deluan/rest" - - . "github.com/Masterminds/squirrel" - "github.com/navidrome/navidrome/model" - "github.com/navidrome/navidrome/model/id" - "github.com/pocketbase/dbx" -) - -type apiKeyRepository struct { - sqlRepository -} - -func NewAPIKeyRepository(ctx context.Context, db dbx.Builder) model.APIKeyRepository { - r := &apiKeyRepository{} - r.ctx = ctx - r.db = db - r.registerModel(&model.APIKey{}, nil) - return r -} - -func (r *apiKeyRepository) userFilter() Sqlizer { - user := loggedUser(r.ctx) - if user.IsAdmin { - return And{} - } - return Eq{"p.user_id": user.ID} -} - -func (r *apiKeyRepository) CountAll(options ...model.QueryOptions) (int64, error) { - sq := r.selectAPIKey(options...).Where(r.userFilter()) - return r.count(sq, options...) -} - -func (r *apiKeyRepository) Get(id string) (*model.APIKey, error) { - sel := r.selectAPIKey().Where(And{Eq{"ak.id": id}}) - var res model.APIKey - err := r.queryOne(sel, &res) - if err != nil { - return nil, err - } - return &res, err -} - -func (r *apiKeyRepository) selectAPIKey(options ...model.QueryOptions) SelectBuilder { - return r.newSelect(options...). - From("api_key ak"). - LeftJoin("player p ON ak.player_id = p.id"). - Columns("ak.*") -} - -func (r *apiKeyRepository) GetAll(options ...model.QueryOptions) (model.APIKeys, error) { - sel := r.selectAPIKey().Where(r.userFilter()) - res := model.APIKeys{} - err := r.queryAll(sel, &res) - if err != nil { - return nil, err - } - return res, err -} - -func (r *apiKeyRepository) Count(options ...rest.QueryOptions) (int64, error) { - return r.CountAll(r.parseRestOptions(r.ctx, options...)) -} - -func (r *apiKeyRepository) Read(id string) (interface{}, error) { - apiKey, err := r.Get(id) - if err != nil { - return nil, err - } - if err := r.VerifyPlayerAccess(apiKey.PlayerID); err != nil { - return nil, err - } - return apiKey, err -} - -func (r *apiKeyRepository) ReadAll(options ...rest.QueryOptions) (interface{}, error) { - return r.GetAll(r.parseRestOptions(r.ctx, options...)) -} - -func (r *apiKeyRepository) EntityName() string { - return "apikey" -} - -func (r *apiKeyRepository) NewInstance() interface{} { - return &model.APIKey{} -} - -func (r *apiKeyRepository) Save(entity interface{}) (string, error) { - ak := entity.(*model.APIKey) - if err := r.VerifyPlayerAccess(ak.PlayerID); err != nil { - return "", err - } - - if ak.ID == "" { - ak.ID = id.NewRandom() - } - ak.Key = generateAPIKey() - ak.CreatedAt = time.Now() - values, err := toSQLArgs(*ak) - if err != nil { - return "", err - } - - insert := Insert(r.tableName).SetMap(values) - _, err = r.executeSQL(insert) - return ak.ID, err -} - -func (r *apiKeyRepository) Update(id string, entity interface{}, _ ...string) error { - ak := entity.(*model.APIKey) - current, err := r.Get(id) - if err != nil { - return err - } - - if err := r.VerifyPlayerAccess(current.PlayerID); err != nil { - return err - } - - update := Update(r.tableName). - Set("name", ak.Name). - Where(Eq{"id": id}) - _, err = r.executeSQL(update) - return err -} - -func (r *apiKeyRepository) Delete(id string) error { - apiKey, err := r.Get(id) - if err != nil { - return err - } - - if err := r.VerifyPlayerAccess(apiKey.PlayerID); err != nil { - return err - } - - return r.delete(Eq{"id": id}) -} - -func (r *apiKeyRepository) FindByKey(key string) (*model.APIKey, error) { - sel := r.selectAPIKey().Where(And{Eq{"ak.key": key}}) - var res model.APIKey - err := r.queryOne(sel, &res) - if err != nil { - return nil, err - } - return &res, err -} - -func (r *apiKeyRepository) RefreshKey(id string) (string, error) { - apiKey, err := r.Get(id) - if err != nil { - return "", err - } - if err := r.VerifyPlayerAccess(apiKey.PlayerID); err != nil { - return "", err - } - - newKey := generateAPIKey() - update := Update(r.tableName). - Set("key", newKey). - Where(Eq{"id": id}) - _, err = r.executeSQL(update) - - if err != nil { - return "", err - } - - return newKey, nil -} - -func (r *apiKeyRepository) VerifyPlayerAccess(playerID string) error { - if playerID == "" { - return model.ErrNotFound - } - - playerRepo := NewPlayerRepository(r.ctx, r.db) - player, err := playerRepo.Get(playerID) - if err != nil { - return err - } - - user := loggedUser(r.ctx) - if !user.IsAdmin && player.UserId != user.ID { - return rest.ErrPermissionDenied - } - - return nil -} - -func generateAPIKey() string { - return "nav_" + id.NewRandom() -} - -var _ model.APIKeyRepository = (*apiKeyRepository)(nil) -var _ rest.Repository = (*apiKeyRepository)(nil) -var _ rest.Persistable = (*apiKeyRepository)(nil) diff --git a/persistence/api_key_repository_test.go b/persistence/api_key_repository_test.go deleted file mode 100644 index d7df1504b..000000000 --- a/persistence/api_key_repository_test.go +++ /dev/null @@ -1,245 +0,0 @@ -package persistence - -import ( - "context" - "github.com/deluan/rest" - "github.com/navidrome/navidrome/log" - "github.com/navidrome/navidrome/model" - "github.com/navidrome/navidrome/model/request" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - "github.com/pocketbase/dbx" -) - -var _ = Describe("APIKeyRepository", func() { - var repo model.APIKeyRepository - var playerRepo model.PlayerRepository - var database *dbx.DB - - var ( - adminPlayer = model.Player{ID: "1", Name: "NavidromeUI [Firefox/Linux]", UserAgent: "Firefox/Linux", UserId: adminUser.ID, Username: adminUser.UserName, Client: "NavidromeUI", IP: "127.0.0.1", ReportRealPath: true, ScrobbleEnabled: true} - regularPlayer = model.Player{ID: "3", Name: "NavidromeUI [Safari/macOS]", UserAgent: "Safari/macOS", UserId: regularUser.ID, Username: regularUser.UserName, Client: "NavidromeUI", ReportRealPath: true, ScrobbleEnabled: false} - - players = model.Players{adminPlayer, regularPlayer} - ) - - BeforeEach(func() { - ctx := log.NewContext(context.TODO()) - ctx = request.WithUser(ctx, adminUser) - database = GetDBXBuilder() - - playerRepo = NewPlayerRepository(ctx, database) - for idx := range players { - err := playerRepo.Put(&players[idx]) - Expect(err).To(BeNil()) - } - repo = NewAPIKeyRepository(ctx, database) - }) - - Describe("FindByKey", func() { - It("returns the API key with matching key", func() { - apiKey := &model.APIKey{ - PlayerID: adminPlayer.ID, - Name: "Unique API Key", - } - apiKeyId, err := repo.Save(apiKey) - Expect(err).ToNot(HaveOccurred()) - apiKey, err = repo.Get(apiKeyId) - Expect(err).ToNot(HaveOccurred()) - - result, err := repo.FindByKey(apiKey.Key) - Expect(err).ToNot(HaveOccurred()) - Expect(result.ID).To(Equal(apiKey.ID)) - Expect(result.Key).To(Equal(apiKey.Key)) - }) - - It("returns error when key not found", func() { - _, err := repo.FindByKey("non-existent-key") - Expect(err).To(HaveOccurred()) - }) - }) - - Describe("Save", func() { - It("creates a new API key with a generated key", func() { - apiKey := &model.APIKey{ - Name: "Test API Key Save", - PlayerID: adminPlayer.ID, - } - - id, err := repo.Save(apiKey) - - Expect(err).ToNot(HaveOccurred()) - Expect(id).ToNot(BeEmpty()) - Expect(apiKey.Key).To(HavePrefix("nav_")) - Expect(apiKey.PlayerID).To(Equal(adminPlayer.ID)) - Expect(apiKey.Name).To(Equal("Test API Key Save")) - }) - }) - - Describe("Update", func() { - It("only updates the name field", func() { - apiKey := &model.APIKey{ - PlayerID: adminPlayer.ID, - Name: "Original Name", - } - - _, err := repo.Save(apiKey) - Expect(err).ToNot(HaveOccurred()) - - updateKey := &model.APIKey{ - Name: "Updated Name", - PlayerID: regularPlayer.ID, - } - - err = repo.Update(apiKey.ID, updateKey) - Expect(err).ToNot(HaveOccurred()) - - result, err := repo.Get(apiKey.ID) - Expect(err).ToNot(HaveOccurred()) - Expect(result.Name).To(Equal("Updated Name")) - Expect(result.Key).To(Equal(apiKey.Key)) - Expect(result.PlayerID).To(Equal(adminPlayer.ID)) - }) - - It("returns error when attempting to update non-existent key", func() { - err := repo.Update("non-existent-id", &model.APIKey{Name: "Updated Name"}) - Expect(err).To(HaveOccurred()) - }) - }) - - Describe("Delete", func() { - It("deletes an existing API key", func() { - apiKey := &model.APIKey{ - PlayerID: adminPlayer.ID, - Name: "API Key to Delete", - } - - _, err := repo.Save(apiKey) - Expect(err).ToNot(HaveOccurred()) - - err = repo.Delete(apiKey.ID) - Expect(err).ToNot(HaveOccurred()) - - _, err = repo.Get(apiKey.ID) - Expect(err).To(HaveOccurred()) - }) - }) - - Describe("RefreshKey", func() { - It("generates a new key for an existing API key", func() { - apiKey := &model.APIKey{ - PlayerID: adminPlayer.ID, - Name: "Test Refresh", - } - _, err := repo.Save(apiKey) - Expect(err).ToNot(HaveOccurred()) - - originalKey := apiKey.Key - - newKey, err := repo.RefreshKey(apiKey.ID) - - Expect(err).ToNot(HaveOccurred()) - Expect(newKey).ToNot(BeEmpty()) - Expect(newKey).ToNot(Equal(originalKey)) - Expect(newKey).To(HavePrefix("nav_")) - - refreshed, err := repo.Get(apiKey.ID) - Expect(err).ToNot(HaveOccurred()) - Expect(refreshed.Key).To(Equal(newKey)) - }) - - It("returns an error for non-existent API key", func() { - _, err := repo.RefreshKey("non-existent-id") - Expect(err).To(HaveOccurred()) - Expect(err).To(MatchError(rest.ErrNotFound)) - }) - - It("enforces user permissions", func() { - apiKey := &model.APIKey{ - PlayerID: adminPlayer.ID, - Name: "Test Permission", - } - _, err := repo.Save(apiKey) - Expect(err).ToNot(HaveOccurred()) - - nonAdminCtx := log.NewContext(context.TODO()) - nonAdminCtx = request.WithUser(nonAdminCtx, regularUser) - nonAdminRepo := NewAPIKeyRepository(nonAdminCtx, database) - - _, err = nonAdminRepo.RefreshKey(apiKey.ID) - Expect(err).To(HaveOccurred()) - Expect(err).To(MatchError(rest.ErrPermissionDenied)) - }) - }) - - Describe("User permissions", func() { - var nonAdminCtx context.Context - var nonAdminRepo model.APIKeyRepository - var adminKey model.APIKey - - BeforeEach(func() { - nonAdminCtx = log.NewContext(context.TODO()) - nonAdminCtx = request.WithUser(nonAdminCtx, regularUser) - nonAdminRepo = NewAPIKeyRepository(nonAdminCtx, database) - - cleanupKeys := func(key string) { - foundKey, err := repo.FindByKey(key) - if err == nil { - _ = repo.Delete(foundKey.ID) - } - } - cleanupKeys("admin-key") - cleanupKeys("user-key") - - tmpAdminKey := &model.APIKey{ - PlayerID: adminPlayer.ID, - Name: "Admin's API Key", - } - _, err := repo.Save(tmpAdminKey) - Expect(err).ToNot(HaveOccurred()) - adminKey = *tmpAdminKey - - userKey := &model.APIKey{ - PlayerID: regularPlayer.ID, - Name: "User's API Key", - } - _, err = repo.Save(userKey) - Expect(err).ToNot(HaveOccurred()) - }) - - It("non-admin users can only see their own API keys", func() { - results, err := nonAdminRepo.GetAll() - Expect(err).ToNot(HaveOccurred()) - - for _, key := range results { - Expect(key.PlayerID).To(Equal(regularPlayer.ID)) - } - }) - - It("admin users can see all API keys", func() { - results, err := repo.GetAll() - Expect(err).ToNot(HaveOccurred()) - - userIds := make(map[string]bool) - for _, key := range results { - userIds[key.PlayerID] = true - } - - Expect(userIds).To(HaveKey(adminPlayer.ID)) - Expect(userIds).To(HaveKey(regularPlayer.ID)) - }) - - It("a user cannot view/delete/update another user's key", func() { - result, err := nonAdminRepo.Read(adminKey.ID) - Expect(result).To(BeNil()) - Expect(err).To(MatchError(rest.ErrPermissionDenied)) - - updatedKey := &model.APIKey{Name: "new admin key name"} - err = nonAdminRepo.Update(adminKey.ID, updatedKey) - Expect(err).To(MatchError(rest.ErrPermissionDenied)) - - err = nonAdminRepo.Delete(adminKey.ID) - Expect(err).To(MatchError(rest.ErrPermissionDenied)) - }) - }) -}) diff --git a/persistence/persistence.go b/persistence/persistence.go index 76f89e7ec..ac607f85f 100644 --- a/persistence/persistence.go +++ b/persistence/persistence.go @@ -89,10 +89,6 @@ func (s *SQLStore) ScrobbleBuffer(ctx context.Context) model.ScrobbleBufferRepos return NewScrobbleBufferRepository(ctx, s.getDBXBuilder()) } -func (s *SQLStore) APIKey(ctx context.Context) model.APIKeyRepository { - return NewAPIKeyRepository(ctx, s.getDBXBuilder()) -} - func (s *SQLStore) Resource(ctx context.Context, m interface{}) model.ResourceRepository { switch m.(type) { case model.User: @@ -117,8 +113,6 @@ func (s *SQLStore) Resource(ctx context.Context, m interface{}) model.ResourceRe return s.Share(ctx).(model.ResourceRepository) case model.Tag: return s.Tag(ctx).(model.ResourceRepository) - case model.APIKey: - return s.APIKey(ctx).(model.ResourceRepository) } log.Error("Resource not implemented", "model", reflect.TypeOf(m).Name()) return nil diff --git a/persistence/player_repository.go b/persistence/player_repository.go index 73c820753..00c5d1fee 100644 --- a/persistence/player_repository.go +++ b/persistence/player_repository.go @@ -7,6 +7,7 @@ import ( . "github.com/Masterminds/squirrel" "github.com/deluan/rest" "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/model/id" "github.com/pocketbase/dbx" ) @@ -57,6 +58,19 @@ func (r *playerRepository) FindMatch(userId, client, userAgent string) (*model.P return &res, err } +func (r *playerRepository) FindByAPIKey(key string) (*model.Player, error) { + if key == "" { + return nil, model.ErrNotFound + } + sel := r.selectPlayer().Where(Eq{"api_key": key}) + var res model.Player + err := r.queryOne(sel, &res) + if err != nil { + return nil, err + } + return &res, nil +} + func (r *playerRepository) newRestSelect(options ...model.QueryOptions) SelectBuilder { s := r.selectPlayer(options...) return s.Where(r.addRestriction()) @@ -130,25 +144,37 @@ func (r *playerRepository) isPermitted(p *model.Player) bool { return u.IsAdmin || p.UserId == u.ID } +func (r *playerRepository) preparePlayerForSave(p *model.Player) *model.Player { + playerCopy := *p + if playerCopy.UserId == "" { + user := loggedUser(r.ctx) + playerCopy.UserId = user.ID + } + playerCopy.APIKey = "" + return &playerCopy +} + func (r *playerRepository) Save(entity interface{}) (string, error) { t := entity.(*model.Player) - if !r.isPermitted(t) { + playerToSave := r.preparePlayerForSave(t) + if !r.isPermitted(playerToSave) { return "", rest.ErrPermissionDenied } - id, err := r.put(t.ID, t) + playerId, err := r.put(playerToSave.ID, playerToSave) if errors.Is(err, model.ErrNotFound) { return "", rest.ErrNotFound } - return id, err + return playerId, err } func (r *playerRepository) Update(id string, entity interface{}, cols ...string) error { t := entity.(*model.Player) t.ID = id - if !r.isPermitted(t) { + playerToUpdate := r.preparePlayerForSave(t) + if !r.isPermitted(playerToUpdate) { return rest.ErrPermissionDenied } - _, err := r.put(id, t, cols...) + _, err := r.put(id, playerToUpdate, cols...) if errors.Is(err, model.ErrNotFound) { return rest.ErrNotFound } @@ -164,6 +190,33 @@ func (r *playerRepository) Delete(id string) error { return err } +// GenerateAPIKey generates a new API key for the specified player +func (r *playerRepository) GenerateAPIKey(playerID string) (string, error) { + player, err := r.Get(playerID) + if errors.Is(err, model.ErrNotFound) { + return "", rest.ErrNotFound + } + if err != nil { + return "", err + } + + if !r.isPermitted(player) { + return "", rest.ErrPermissionDenied + } + + player.APIKey = generateAPIKey() + err = r.Put(player) + if err != nil { + return "", err + } + return player.APIKey, nil +} + +// generateAPIKey creates a new random API key +func generateAPIKey() string { + return "nav_" + id.NewRandom() +} + var _ model.PlayerRepository = (*playerRepository)(nil) var _ rest.Repository = (*playerRepository)(nil) var _ rest.Persistable = (*playerRepository)(nil) diff --git a/persistence/player_repository_test.go b/persistence/player_repository_test.go index f6c669493..f7e71f6b4 100644 --- a/persistence/player_repository_test.go +++ b/persistence/player_repository_test.go @@ -13,8 +13,12 @@ import ( ) var _ = Describe("PlayerRepository", func() { - var adminRepo *playerRepository - var database *dbx.DB + var ( + adminRepo *playerRepository + database *dbx.DB + adminCtx context.Context + regularCtx context.Context + ) var ( adminPlayer1 = model.Player{ID: "1", Name: "NavidromeUI [Firefox/Linux]", UserAgent: "Firefox/Linux", UserId: adminUser.ID, Username: adminUser.UserName, Client: "NavidromeUI", IP: "127.0.0.1", ReportRealPath: true, ScrobbleEnabled: true} @@ -25,11 +29,14 @@ var _ = Describe("PlayerRepository", func() { ) BeforeEach(func() { - ctx := log.NewContext(context.TODO()) - ctx = request.WithUser(ctx, adminUser) + adminCtx = log.NewContext(context.TODO()) + adminCtx = request.WithUser(adminCtx, adminUser) + + regularCtx = log.NewContext(context.TODO()) + regularCtx = request.WithUser(regularCtx, regularUser) database = GetDBXBuilder() - adminRepo = NewPlayerRepository(ctx, database).(*playerRepository) + adminRepo = NewPlayerRepository(adminCtx, database).(*playerRepository) for idx := range players { err := adminRepo.Put(&players[idx]) @@ -86,6 +93,28 @@ var _ = Describe("PlayerRepository", func() { }) }) + Describe("Save", func() { + var repo *playerRepository + BeforeEach(func() { + ctx := log.NewContext(context.TODO()) + ctx = request.WithUser(ctx, regularUser) + repo = NewPlayerRepository(ctx, database).(*playerRepository) + }) + It("fails if different user id is set", func() { + newPlayer := model.Player{ + Name: "Test Player", + Client: "TestClient", + UserAgent: "TestAgent", + UserId: adminUser.ID, + } + id, err := repo.Save(&newPlayer) + Expect(err).To(Equal(rest.ErrPermissionDenied)) + + _, err = repo.Get(id) + Expect(err).To(Equal(rest.ErrNotFound)) + }) + }) + DescribeTableSubtree("per context", func(admin bool, players model.Players, userPlayer model.Player, otherPlayer model.Player) { var repo *playerRepository @@ -171,41 +200,37 @@ var _ = Describe("PlayerRepository", func() { }) Describe("Save", func() { - DescribeTable("item type", func(player model.Player) { - clone := player - clone.ID = "" - clone.IP = "192.168.1.1" - id, err := repo.Save(&clone) - - if clone.UserId == "" { - Expect(err).To(HaveOccurred()) - } else if !admin && player.Username == adminPlayer1.Username { - Expect(err).To(Equal(rest.ErrPermissionDenied)) - clone.UserId = "" - } else { - Expect(err).To(BeNil()) - Expect(id).ToNot(BeEmpty()) + It("excludes API key from being saved", func() { + newPlayer := model.Player{ + Name: "Test Player", + Client: "TestClient", + UserAgent: "TestAgent", + APIKey: "should-not-be-saved", } - count, err := repo.Count() + id, err := repo.Save(&newPlayer) Expect(err).To(BeNil()) - clone.ID = id - newItem, err := repo.Get(id) + savedPlayer, err := repo.Get(id) + Expect(err).To(BeNil()) + Expect(savedPlayer.APIKey).To(BeEmpty()) + }) - if clone.UserId == "" { - Expect(count).To(Equal(baseCount)) - Expect(err).To(Equal(model.ErrNotFound)) - } else { - Expect(count).To(Equal(baseCount + 1)) - Expect(err).To(BeNil()) - Expect(*newItem).To(Equal(clone)) + It("sets user ID from context", func() { + repo := NewPlayerRepository(regularCtx, database).(*playerRepository) + newPlayer := model.Player{ + Name: "Test Player", + Client: "TestClient", + UserAgent: "TestAgent", } - }, - Entry("same user", userPlayer), - Entry("other item", otherPlayer), - Entry("fake item", model.Player{}), - ) + + id, err := repo.Save(&newPlayer) + Expect(err).To(BeNil()) + + savedPlayer, err := repo.Get(id) + Expect(err).To(BeNil()) + Expect(savedPlayer.UserId).To(Equal(regularUser.ID)) + }) }) Describe("Update", func() { @@ -215,9 +240,7 @@ var _ = Describe("PlayerRepository", func() { clone.MaxBitRate = 10000 err := repo.Update(clone.ID, &clone, "ip") - if clone.UserId == "" { - Expect(err).To(HaveOccurred()) - } else if !admin && player.Username == adminPlayer1.Username { + if !admin && player.Username == adminPlayer1.Username { Expect(err).To(Equal(rest.ErrPermissionDenied)) clone.IP = player.IP } else { diff --git a/persistence/user_repository.go b/persistence/user_repository.go index ee057e373..a7181b1a7 100644 --- a/persistence/user_repository.go +++ b/persistence/user_repository.go @@ -193,21 +193,6 @@ func (r *userRepository) FindByUsernameWithPassword(username string) (*model.Use return usr, nil } -func (r *userRepository) FindByAPIKey(key string) (*model.User, error) { - // find the API key in the database - playerRepo := NewPlayerRepository(r.ctx, r.db) - apiKeyRepo := NewAPIKeyRepository(r.ctx, r.db) - apiKey, err := apiKeyRepo.FindByKey(key) - if err != nil { - return nil, err - } - player, err := playerRepo.Get(apiKey.PlayerID) - if err != nil { - return nil, err - } - return r.Get(player.UserId) -} - func (r *userRepository) UpdateLastLoginAt(id string) error { upd := Update(r.tableName).Where(Eq{"id": id}).Set("last_login_at", time.Now()) _, err := r.executeSQL(upd) diff --git a/persistence/user_repository_test.go b/persistence/user_repository_test.go index 615c336f5..7c0707ecd 100644 --- a/persistence/user_repository_test.go +++ b/persistence/user_repository_test.go @@ -212,7 +212,7 @@ var _ = Describe("UserRepository", func() { var existingUser *model.User BeforeEach(func() { existingUser = &model.User{ID: "1", UserName: "johndoe"} - repo = tests.CreateMockUserRepo(nil, nil) + repo = tests.CreateMockUserRepo() err := repo.Put(existingUser) Expect(err).ToNot(HaveOccurred()) }) diff --git a/server/nativeapi/native_api.go b/server/nativeapi/native_api.go index cab45fd6a..97509657e 100644 --- a/server/nativeapi/native_api.go +++ b/server/nativeapi/native_api.go @@ -3,8 +3,7 @@ package nativeapi import ( "context" "encoding/json" - "fmt" - "github.com/navidrome/navidrome/utils/req" + "errors" "html" "net/http" "strconv" @@ -53,11 +52,10 @@ func (n *Router) routes() http.Handler { n.R(r, "/album", model.Album{}, false) n.R(r, "/artist", model.Artist{}, false) n.R(r, "/genre", model.Genre{}, false) - n.R(r, "/player", model.Player{}, true) + n.addPlayerRoute(r) n.R(r, "/transcoding", model.Transcoding{}, conf.Server.EnableTranscodingConfig) n.R(r, "/radio", model.Radio{}, true) n.R(r, "/tag", model.Tag{}, true) - n.R(r, "/apikey", model.APIKey{}, true) if conf.Server.EnableSharing { n.RX(r, "/share", n.share.NewRepository, true) } @@ -76,7 +74,6 @@ func (n *Router) routes() http.Handler { n.addUserLibraryRoute(r) n.RX(r, "/library", n.libs.NewRepository, true) }) - n.addRefreshApiKeyRoute(r) }) return r @@ -106,6 +103,25 @@ func (n *Router) RX(r chi.Router, pathPrefix string, constructor rest.Repository }) } +func (n *Router) addPlayerRoute(r chi.Router) { + constructor := func(ctx context.Context) rest.Repository { + return n.ds.Resource(ctx, model.Player{}) + } + + r.Route("/player", func(r chi.Router) { + r.Get("/", rest.GetAll(constructor)) + r.Post("/", rest.Post(constructor)) + + r.Route("/{id}", func(r chi.Router) { + r.Use(server.URLParamsMiddleware) + r.Get("/", rest.Get(constructor)) + r.Put("/", rest.Put(constructor)) + r.Delete("/", rest.Delete(constructor)) + r.Post("/apiKey", generatePlayerApiKey(n.ds)) + }) + }) +} + func (n *Router) addPlaylistRoute(r chi.Router) { constructor := func(ctx context.Context) rest.Repository { return n.ds.Resource(ctx, model.Playlist{}) @@ -202,6 +218,28 @@ func writeDeleteManyResponse(w http.ResponseWriter, r *http.Request, ids []strin } } +func generatePlayerApiKey(ds model.DataStore) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + playerId := chi.URLParam(r, "id") + err := ds.WithTxImmediate(func(tx model.DataStore) error { + repo := tx.Player(ctx) + apiKey, err := repo.GenerateAPIKey(playerId) + if err == nil { + resp := []byte(`{"apiKey":"` + html.EscapeString(apiKey) + `"}`) + _, err = w.Write(resp) + } + return err + }) + if errors.Is(err, model.ErrNotFound) { + http.Error(w, "not found", http.StatusNotFound) + } else if err != nil { + log.Error(ctx, "Error retrieving player from DB", "id", playerId, err) + http.Error(w, err.Error(), http.StatusInternalServerError) + } + } +} + func (n *Router) addInspectRoute(r chi.Router) { if conf.Server.Inspect.Enabled { r.Group(func(r chi.Router) { @@ -250,37 +288,3 @@ func adminOnlyMiddleware(next http.Handler) http.Handler { next.ServeHTTP(w, r) }) } - -func (n *Router) addRefreshApiKeyRoute(r chi.Router) { - r.With(server.URLParamsMiddleware).Post("/apikey/{id}/refresh", func(w http.ResponseWriter, r *http.Request) { - p := req.Params(r) - id, err := p.String(":id") - if err != nil { - msg := fmt.Sprintf("api key id could not be parsed: %s", id) - log.Warn(msg) - http.Error(w, "not found", http.StatusNotFound) - return - } - - repo := n.ds.APIKey(r.Context()) - _, err = repo.RefreshKey(id) - if err != nil { - log.Error(r.Context(), "error refreshing api key", "id", id, err) - http.Error(w, err.Error(), http.StatusInternalServerError) - return - } - - updatedKey, err := repo.Get(id) - if err != nil { - log.Error(r.Context(), "error retrieving refreshed api key", "id", id, err) - http.Error(w, err.Error(), http.StatusInternalServerError) - return - } - err = rest.RespondWithJSON(w, http.StatusOK, updatedKey) - if err != nil { - log.Error(r.Context(), "error marshaling refreshed api key", "id", id, err) - http.Error(w, err.Error(), http.StatusInternalServerError) - return - } - }) -} diff --git a/server/subsonic/middlewares.go b/server/subsonic/middlewares.go index a39063273..3ea68cd9c 100644 --- a/server/subsonic/middlewares.go +++ b/server/subsonic/middlewares.go @@ -63,6 +63,7 @@ func checkRequiredParameters(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { var requiredParameters []string + p := req.Params(r) username, _ := fromInternalOrProxyAuth(r) if username != "" { requiredParameters = []string{"v", "c"} @@ -154,15 +155,22 @@ func authenticate(ds model.DataStore) func(next http.Handler) http.Handler { } if apiKey != "" { - usr, err = ds.User(ctx).FindByAPIKey(apiKey) + var player *model.Player + player, err = ds.Player(ctx).FindByAPIKey(apiKey) if errors.Is(err, context.Canceled) { - log.Debug(ctx, "API: Request canceled when authenticating", "auth", "subsonic-apikey", "remoteAddr", r.RemoteAddr, err) + log.Debug(ctx, "API: Request canceled when authenticating", "auth", "subsonic", "remoteAddr", r.RemoteAddr, err) return } - if errors.Is(err, model.ErrNotFound) { - log.Warn(ctx, "API: Invalid login - API key not found", "auth", "subsonic-apikey", "remoteAddr", r.RemoteAddr) - } else if err != nil { - log.Error(ctx, "API: Error authenticating with API key", "auth", "subsonic-apikey", "remoteAddr", r.RemoteAddr, err) + switch { + case errors.Is(err, model.ErrNotFound): + log.Warn(ctx, "API: Invalid login - API key not found", "auth", "subsonic", "apikey", apiKey, "remoteAddr", r.RemoteAddr, err) + case err != nil: + log.Error(ctx, "API: Error authenticating with API key", "auth", "subsonic", "apikey", apiKey, "remoteAddr", r.RemoteAddr, err) + default: + usr, err = ds.User(ctx).Get(player.UserId) + if err != nil { + log.Error(ctx, "API: Error retrieving user from API key", "auth", "subsonic", "apikey", apiKey, "remoteAddr", r.RemoteAddr, err) + } } } else { usr, err = ds.User(ctx).FindByUsernameWithPassword(username) diff --git a/server/subsonic/middlewares_test.go b/server/subsonic/middlewares_test.go index ee3879c09..33c38404d 100644 --- a/server/subsonic/middlewares_test.go +++ b/server/subsonic/middlewares_test.go @@ -308,7 +308,6 @@ var _ = Describe("Middlewares", func() { }) When("using api key authentication", func() { - var apiKey *model.APIKey BeforeEach(func() { DeferCleanup(configtest.SetupConfig()) @@ -333,22 +332,13 @@ var _ = Describe("Middlewares", func() { MaxBitRate: 320, ReportRealPath: false, ScrobbleEnabled: true, + APIKey: "nav_12345", } _ = pr.Put(player) - - ar := ds.APIKey(context.TODO()) - newApiKey := &model.APIKey{ - ID: "api-key-id", - Name: "API Key", - PlayerID: player.ID, - } - apiKeyId, _ := ar.Save(newApiKey) - newApiKey, _ = ar.Get(apiKeyId) - apiKey = newApiKey }) It("passes authentication with correct api key", func() { - r := newGetRequest("apiKey=" + apiKey.Key) + r := newGetRequest("apiKey=nav_12345") cp := authenticate(ds)(next) cp.ServeHTTP(w, r) diff --git a/tests/mock_apikey_repo.go b/tests/mock_apikey_repo.go deleted file mode 100644 index b99f2316c..000000000 --- a/tests/mock_apikey_repo.go +++ /dev/null @@ -1,60 +0,0 @@ -package tests - -import ( - "github.com/navidrome/navidrome/model" - "github.com/navidrome/navidrome/model/id" - "strings" -) - -func CreateMockApiKeyRepo() *MockedAPIKeyRepo { - return &MockedAPIKeyRepo{ - Data: map[string]*model.APIKey{}, - } -} - -type MockedAPIKeyRepo struct { - model.APIKeyRepository - Error error - Data map[string]*model.APIKey -} - -func (m *MockedAPIKeyRepo) CountAll(_ ...model.QueryOptions) (int64, error) { - if m.Error != nil { - return 0, m.Error - } - return int64(len(m.Data)), nil -} - -func (m *MockedAPIKeyRepo) Save(entity interface{}) (string, error) { - if m.Error != nil { - return "", m.Error - } - apiKey := entity.(*model.APIKey) - apiKey.Key = "nav_" + id.NewRandom() - m.Data[strings.ToLower(apiKey.Key)] = apiKey - return apiKey.ID, nil -} - -func (m *MockedAPIKeyRepo) FindByKey(key string) (*model.APIKey, error) { - if m.Error != nil { - return nil, m.Error - } - apiKey, exists := m.Data[strings.ToLower(key)] - if !exists { - return nil, model.ErrNotFound - } - return apiKey, nil -} - -func (m *MockedAPIKeyRepo) Get(id string) (*model.APIKey, error) { - if m.Error != nil { - return nil, m.Error - } - - for _, apiKey := range m.Data { - if apiKey.ID == id { - return apiKey, nil - } - } - return nil, model.ErrNotFound -} diff --git a/tests/mock_data_store.go b/tests/mock_data_store.go index 5945c7faa..d05795d70 100644 --- a/tests/mock_data_store.go +++ b/tests/mock_data_store.go @@ -28,7 +28,6 @@ type MockDataStore struct { MockedRadio model.RadioRepository scrobbleBufferMu sync.Mutex repoMu sync.Mutex - MockedAPIKey model.APIKeyRepository } func (db *MockDataStore) Library(ctx context.Context) model.LibraryRepository { @@ -170,9 +169,7 @@ func (db *MockDataStore) User(ctx context.Context) model.UserRepository { if db.RealDS != nil { db.MockedUser = db.RealDS.User(ctx) } else { - playerRepo := db.Player(ctx).(*MockedPlayerRepo) - apiKeyRepo := db.APIKey(ctx).(*MockedAPIKeyRepo) - db.MockedUser = CreateMockUserRepo(playerRepo, apiKeyRepo) + db.MockedUser = CreateMockUserRepo() } } return db.MockedUser @@ -224,17 +221,6 @@ func (db *MockDataStore) Radio(ctx context.Context) model.RadioRepository { return db.MockedRadio } -func (db *MockDataStore) APIKey(ctx context.Context) model.APIKeyRepository { - if db.MockedAPIKey == nil { - if db.RealDS != nil { - db.MockedAPIKey = db.RealDS.APIKey(ctx) - } else { - db.MockedAPIKey = CreateMockApiKeyRepo() - } - } - return db.MockedAPIKey -} - func (db *MockDataStore) WithTx(block func(tx model.DataStore) error, label ...string) error { return block(db) } diff --git a/tests/mock_player_repo.go b/tests/mock_player_repo.go index b84cc2663..7d5c6dd1e 100644 --- a/tests/mock_player_repo.go +++ b/tests/mock_player_repo.go @@ -2,6 +2,7 @@ package tests import ( "encoding/base64" + "github.com/navidrome/navidrome/model" ) @@ -72,3 +73,16 @@ func (m *MockedPlayerRepo) CountByClient(_ ...model.QueryOptions) (map[string]in } return result, nil } + +func (m *MockedPlayerRepo) FindByAPIKey(key string) (*model.Player, error) { + if m.Error != nil { + return nil, m.Error + } + + for _, player := range m.Data { + if player.APIKey == key { + return player, nil + } + } + return nil, model.ErrNotFound +} diff --git a/tests/mock_user_repo.go b/tests/mock_user_repo.go index bc88fc04c..ef4ad8508 100644 --- a/tests/mock_user_repo.go +++ b/tests/mock_user_repo.go @@ -10,17 +10,10 @@ import ( "github.com/navidrome/navidrome/utils/gg" ) -func CreateMockUserRepo(apiKeyRepo ...*MockedAPIKeyRepo) *MockedUserRepo { - var repo *MockedAPIKeyRepo - if len(apiKeyRepo) > 0 { - repo = apiKeyRepo[0] - } else { - repo = CreateMockApiKeyRepo() - } +func CreateMockUserRepo() *MockedUserRepo { return &MockedUserRepo{ Data: map[string]*model.User{}, UserLibraries: map[string][]int{}, - APIKeyRepo: repo, } } @@ -29,8 +22,6 @@ type MockedUserRepo struct { Error error Data map[string]*model.User UserLibraries map[string][]int // userID -> libraryIDs - APIKeyRepo *MockedAPIKeyRepo - PlayerRepo *MockedPlayerRepo } func (u *MockedUserRepo) CountAll(_ ...model.QueryOptions) (int64, error) { @@ -132,27 +123,3 @@ func (u *MockedUserRepo) SetUserLibraries(userID string, libraryIDs []int) error u.UserLibraries[userID] = libraryIDs return nil } - -func (u *MockedUserRepo) FindByAPIKey(key string) (*model.User, error) { - if u.Error != nil { - return nil, u.Error - } - - apiKey, err := u.APIKeyRepo.FindByKey(key) - if err != nil { - return nil, err - } - - player, err := u.PlayerRepo.Get(apiKey.PlayerID) - if err != nil { - return nil, err - } - - for _, usr := range u.Data { - if usr.ID == player.UserId { - return usr, nil - } - } - - return nil, model.ErrNotFound -} diff --git a/ui/src/App.jsx b/ui/src/App.jsx index 7a5899045..dc4fe9b53 100644 --- a/ui/src/App.jsx +++ b/ui/src/App.jsx @@ -42,7 +42,6 @@ import SharePlayer from './share/SharePlayer' import { HTML5Backend } from 'react-dnd-html5-backend' import { DndProvider } from 'react-dnd' import missing from './missing/index.js' -import apikey from './apikey/index.js' const history = createHashHistory() @@ -112,11 +111,6 @@ const Admin = (props) => { options={{ subMenu: 'playlist' }} />, <Resource name="user" {...user} options={{ subMenu: 'settings' }} />, - <Resource - name="apikey" - {...apikey} - options={{ subMenu: 'settings' }} - />, <Resource name="player" {...player} diff --git a/ui/src/i18n/en.json b/ui/src/i18n/en.json index 48a8bc819..a0847f7c1 100644 --- a/ui/src/i18n/en.json +++ b/ui/src/i18n/en.json @@ -1,22 +1,6 @@ { "languageName": "English", "resources": { - "apikey": { - "name": "API Key |||| API Keys", - "fields": { - "name": "Name", - "key": "Key", - "createdAt": "Created At", - "key_help_text": "This key will be used for API authentication" - }, - "actions": { - "add": "Add API Key" - }, - "notifications": { - "created": "API Key created", - "updated": "API Key updated" - } - }, "song": { "name": "Song |||| Songs", "fields": { @@ -197,7 +181,8 @@ "userName": "Username", "lastSeen": "Last Seen At", "reportRealPath": "Report Real Path", - "scrobbleEnabled": "Send Scrobbles to external services" + "scrobbleEnabled": "Send Scrobbles to external services", + "apiKey": "API Key" } }, "transcoding": { @@ -524,7 +509,8 @@ "shareSuccess": "URL copied to clipboard: %{url}", "shareFailure": "Error copying URL %{url} to clipboard", "downloadDialogTitle": "Download %{resource} '%{name}' (%{size})", - "downloadOriginalFormat": "Download in original format" + "downloadOriginalFormat": "Download in original format", + "apiKeyGenerated": "API key generated successfully" }, "menu": { "library": "Library", diff --git a/ui/src/player/PlayerCreate.jsx b/ui/src/player/PlayerCreate.jsx new file mode 100644 index 000000000..86b5dc7bd --- /dev/null +++ b/ui/src/player/PlayerCreate.jsx @@ -0,0 +1,44 @@ +import React from 'react' +import { + TextInput, + BooleanInput, + Create, + required, + SimpleForm, + SelectInput, + ReferenceInput, +} from 'react-admin' +import { BITRATE_CHOICES } from '../consts' +import config from '../config.js' +import { Title } from '../common' +import { useTranslate } from 'react-admin' + +const PlayerCreateTitle = () => { + const translate = useTranslate() + const resourceName = translate('resources.player.name', { smart_count: 1 }) + return <Title subTitle={`${translate('ra.action.create')} ${resourceName}`} /> +} + +const PlayerCreate = (props) => { + return ( + <Create title={<PlayerCreateTitle />} {...props}> + <SimpleForm variant="outlined"> + <TextInput source="name" validate={[required()]} /> + <ReferenceInput + source="transcodingId" + reference="transcoding" + sort={{ field: 'name', order: 'ASC' }} + > + <SelectInput source="name" resettable /> + </ReferenceInput> + <SelectInput source="maxBitRate" resettable choices={BITRATE_CHOICES} /> + <BooleanInput source="reportRealPath" fullWidth /> + {(config.lastFMEnabled || config.listenBrainzEnabled) && ( + <BooleanInput source="scrobbleEnabled" fullWidth /> + )} + </SimpleForm> + </Create> + ) +} + +export default PlayerCreate diff --git a/ui/src/player/PlayerEdit.jsx b/ui/src/player/PlayerEdit.jsx index 1826500bd..6154ff5bf 100644 --- a/ui/src/player/PlayerEdit.jsx +++ b/ui/src/player/PlayerEdit.jsx @@ -1,3 +1,4 @@ +import React, { useState, useCallback } from 'react' import { TextInput, BooleanInput, @@ -8,10 +9,25 @@ import { SelectInput, ReferenceInput, useTranslate, + useNotify, + Button, + useRecordContext, } from 'react-admin' +import FileCopyIcon from '@material-ui/icons/FileCopy' +import VpnKeyIcon from '@material-ui/icons/VpnKey' +import VisibilityIcon from '@material-ui/icons/Visibility' +import VisibilityOffIcon from '@material-ui/icons/VisibilityOff' import { Title } from '../common' -import config from '../config' -import { BITRATE_CHOICES } from '../consts' +import { BITRATE_CHOICES, REST_URL } from '../consts' +import { makeStyles } from '@material-ui/core/styles' +import { + IconButton, + InputAdornment, + Tooltip, + TextField as MuiTextField, +} from '@material-ui/core' +import httpClient from '../dataProvider/httpClient.js' +import config from '../config.js' const PlayerTitle = ({ record }) => { const translate = useTranslate() @@ -19,26 +35,148 @@ const PlayerTitle = ({ record }) => { return <Title subTitle={`${resourceName} ${record ? record.name : ''}`} /> } -const PlayerEdit = (props) => ( - <Edit title={<PlayerTitle />} {...props}> - <SimpleForm variant={'outlined'}> - <TextInput source="name" validate={[required()]} /> - <ReferenceInput - source="transcodingId" - reference="transcoding" - sort={{ field: 'name', order: 'ASC' }} - > - <SelectInput source="name" resettable /> - </ReferenceInput> - <SelectInput source="maxBitRate" resettable choices={BITRATE_CHOICES} /> - <BooleanInput source="reportRealPath" fullWidth /> - {(config.lastFMEnabled || config.listenBrainzEnabled) && ( - <BooleanInput source="scrobbleEnabled" fullWidth /> +const useStyles = makeStyles({ + apiKeyField: { + marginTop: '16px', + marginBottom: '8px', + }, + generateButton: { + marginTop: '16px', + marginBottom: '8px', + }, + copyButton: { + padding: 4, + }, +}) + +const ApiKeySection = () => { + const record = useRecordContext() + const recordId = record ? record.id : null + const initialApiKey = record ? record.apiKey : null + + const classes = useStyles() + const notify = useNotify() + const [showApiKey, setShowApiKey] = useState(false) + const [loading, setLoading] = useState(false) + const [apiKey, setApiKey] = useState(initialApiKey) + + const generateApiKey = useCallback(async () => { + if (!recordId) { + notify('Player ID not available', 'error') + return + } + + try { + setLoading(true) + const { json } = await httpClient( + `${REST_URL}/player/${recordId}/apiKey`, + { + method: 'POST', + }, + ) + setApiKey(json.apiKey) + setShowApiKey(true) + notify('message.apiKeyGenerated', 'info') + } catch (error) { + notify(error.message || 'Error generating API key', 'error') + } finally { + setLoading(false) + } + }, [recordId, notify]) + + const copyToClipboard = () => { + if (apiKey) { + navigator.clipboard.writeText(apiKey) + notify('API key copied to clipboard', 'info') + } + } + + if (!recordId) { + return <></> + } + + return ( + <div style={{ display: 'flex', flexDirection: 'column', marginTop: 16 }}> + {apiKey ? ( + <MuiTextField + label="API Key" + value={showApiKey ? apiKey : '*'.repeat(apiKey.length)} + InputProps={{ + readOnly: true, + disableUnderline: true, + endAdornment: ( + <InputAdornment position="end"> + <Tooltip title={showApiKey ? 'Hide' : 'Show'}> + <IconButton + aria-label="toggle api key visibility" + onClick={() => setShowApiKey(!showApiKey)} + size="small" + > + {showApiKey ? <VisibilityOffIcon /> : <VisibilityIcon />} + </IconButton> + </Tooltip> + <Tooltip title="Copy to clipboard"> + <IconButton + aria-label="copy api key" + onClick={copyToClipboard} + className={classes.copyButton} + size="small" + > + <FileCopyIcon /> + </IconButton> + </Tooltip> + </InputAdornment> + ), + }} + style={{ width: 320, color: 'black' }} + /> + ) : ( + !loading && ( + <MuiTextField + label="API Key" + value="No API Key Found" + disabled + style={{ width: 320 }} + /> + ) )} - <TextField source="client" /> - <TextField source="userName" /> - </SimpleForm> - </Edit> -) + + <Button + style={{ marginTop: 12, alignSelf: 'flex-start' }} + onClick={generateApiKey} + label="Generate New API Key" + startIcon={<VpnKeyIcon />} + variant="outlined" + disabled={loading} + /> + </div> + ) +} + +const PlayerEdit = (props) => { + return ( + <Edit title={<PlayerTitle />} {...props}> + <SimpleForm variant={'outlined'}> + <TextInput source="name" validate={[required()]} /> + <ReferenceInput + source="transcodingId" + reference="transcoding" + sort={{ field: 'name', order: 'ASC' }} + > + <SelectInput source="name" resettable /> + </ReferenceInput> + <SelectInput source="maxBitRate" resettable choices={BITRATE_CHOICES} /> + <BooleanInput source="reportRealPath" fullWidth /> + {(config.lastFMEnabled || config.listenBrainzEnabled) && ( + <BooleanInput source="scrobbleEnabled" fullWidth /> + )} + <TextField source="client" /> + <TextField source="userName" /> + + <ApiKeySection /> + </SimpleForm> + </Edit> + ) +} export default PlayerEdit diff --git a/ui/src/player/index.js b/ui/src/player/index.js index aaa3d58d7..6e356266e 100644 --- a/ui/src/player/index.js +++ b/ui/src/player/index.js @@ -1,8 +1,10 @@ import { BsFillMusicPlayerFill } from 'react-icons/bs' import PlayerList from './PlayerList' import PlayerEdit from './PlayerEdit' +import PlayerCreate from './PlayerCreate.jsx' export default { + create: PlayerCreate, list: PlayerList, edit: PlayerEdit, icon: BsFillMusicPlayerFill,