nik/navidrome
1
0
Fork 0
mirror of https://github.com/navidrome/navidrome.git synced 2026-08-01 07:21:17 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix LDAP auth review feedback

Browse Source
This commit is contained in:
Firehawk 2026-07-12 04:26:51 +09:30
parent a33e3c7f9b
commit 8963eeb959
3 changed files with 32 additions and 27 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
core/ldapauth/ldapauth_test.go
View File

@ -12,8 +12,8 @@ func TestLoginUserFilter(t *testing.T) {
t.Run("defaults to username attribute equality", func(t *testing.T) {
t.Parallel()
got := loginUserFilter(Source{UserNameAttribute: "uid"}, "firehawk")
want := "(uid=firehawk)"
got := loginUserFilter(Source{UserNameAttribute: "uid"}, "directory-user")
want := "(uid=directory-user)"
if got != want {
t.Fatalf("loginUserFilter() = %q, want %q", got, want)
}
@ -21,8 +21,8 @@ func TestLoginUserFilter(t *testing.T) {
t.Run("preserves explicit placeholder filters", func(t *testing.T) {
t.Parallel()
got := loginUserFilter(Source{UserNameAttribute: "uid", UserFilter: "(&(objectClass=person)(%s=%s))"}, "firehawk")
want := "(&(objectClass=person)(uid=firehawk))"
got := loginUserFilter(Source{UserNameAttribute: "uid", UserFilter: "(&(objectClass=person)(%s=%s))"}, "directory-user")
want := "(&(objectClass=person)(uid=directory-user))"
if got != want {
t.Fatalf("loginUserFilter() = %q, want %q", got, want)
}
@ -30,8 +30,8 @@ func TestLoginUserFilter(t *testing.T) {
t.Run("supports one-placeholder username filters", func(t *testing.T) {
t.Parallel()
got := loginUserFilter(Source{UserNameAttribute: "uid", UserFilter: "(uid=%s)"}, "firehawk")
want := "(uid=firehawk)"
got := loginUserFilter(Source{UserNameAttribute: "uid", UserFilter: "(uid=%s)"}, "directory-user")
want := "(uid=directory-user)"
if got != want {
t.Fatalf("loginUserFilter() = %q, want %q", got, want)
}
@ -39,8 +39,8 @@ func TestLoginUserFilter(t *testing.T) {
t.Run("adds username assertion to discovery filters", func(t *testing.T) {
t.Parallel()
got := loginUserFilter(Source{UserNameAttribute: "uid", UserFilter: "(objectClass=person)"}, "firehawk")
want := "(&(objectClass=person)(uid=firehawk))"
got := loginUserFilter(Source{UserNameAttribute: "uid", UserFilter: "(objectClass=person)"}, "directory-user")
want := "(&(objectClass=person)(uid=directory-user))"
if got != want {
t.Fatalf("loginUserFilter() = %q, want %q", got, want)
}
@ -65,10 +65,10 @@ func TestAuthInternalSkipsExternalUsersForFallback(t *testing.T) {
t.Parallel()
repo := tests.CreateMockUserRepo()
repo.Data["firehawk"] = &model.User{ID: "1", UserName: "firehawk", Password: "generated", AuthSource: "ldap", AuthSourceID: "freeipa"}
repo.Data["directory-user"] = &model.User{ID: "1", UserName: "directory-user", Password: "generated", AuthSource: "ldap", AuthSourceID: "freeipa"}
ds := &tests.MockDataStore{MockedUser: repo}
user, found, err := authInternal(t.Context(), ds, "firehawk", "ldap-password", true)
user, found, err := authInternal(t.Context(), ds, "directory-user", "ldap-password", true)
if err != nil {
t.Fatalf("authInternal() unexpected error: %v", err)
}
@ -76,7 +76,7 @@ func TestAuthInternalSkipsExternalUsersForFallback(t *testing.T) {
t.Fatalf("authInternal() found=%v user=%#v, want external user to be skipped for fallback", found, user)
}
user, found, err = authInternal(t.Context(), ds, "firehawk", "ldap-password", false)
user, found, err = authInternal(t.Context(), ds, "directory-user", "ldap-password", false)
if err != nil {
t.Fatalf("authInternal() unexpected error: %v", err)
}

2
db/migrations/20260711130000_add_user_auth_source.sql
View File

@ -1,10 +1,12 @@
-- +goose Up
-- Add LDAP ownership metadata to users.
-- +goose StatementBegin
alter table user add column auth_source varchar(32) default '' not null;
alter table user add column auth_source_id varchar(255) default '' not null;
-- +goose StatementEnd
-- +goose Down
-- Roll back the LDAP ownership metadata added in the Up migration.
-- +goose StatementBegin
alter table user drop column auth_source;
alter table user drop column auth_source_id;

35
server/subsonic/middlewares.go
View File

@ -127,25 +127,28 @@ func authenticate(ds model.DataStore) func(next http.Handler) http.Handler {
salt, _ := p.String("s")
jwt, _ := p.String("jwt")
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):
if pass != "" && token == "" && jwt == "" {
usr, err = ldapauth.Authenticate(ctx, ds, "", username, pass)
} else {
if pass != "" && token == "" && jwt == "" {
usr, err = ldapauth.Authenticate(ctx, ds, "", username, pass)
if usr == nil && err == nil {
err = model.ErrInvalidAuth
}
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)
} 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):
err = model.ErrInvalidAuth
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)
}
}