[PR #6669] [management] Trim trailing slash from Keycloak AdminEndpoint #29789

Open
opened 2026-08-05 08:08:58 -04:00 by saavagebueno · 0 comments
Owner

Original Pull Request: https://github.com/netbirdio/netbird/pull/6669

State: open
Merged: No


Summary

  • A trailing slash in the configured Keycloak AdminEndpoint produced a double slash in admin API request paths, e.g. https://<host>/admin/realms/<realm>//users/count?.
  • Keycloak >= 26.4.3 requires normalised admin API request paths and rejects the double slash with a 400, which breaks dashboard login and VPN client authentication entirely (see #4979).
  • Root cause: management/server/idp/keycloak.go builds request URLs with fmt.Sprintf("%s/%s", km.adminEndpoint, resource) in two call sites (get, DeleteUser); if the configured endpoint already ends in /, this yields //.
  • Fix: normalize once at construction time in NewKeycloakManager (strings.TrimRight(config.AdminEndpoint, "/")) instead of patching every call site individually.

Fixes #4979.

Test plan

  • Added TestKeycloakManager_AdminEndpointTrailingSlash (red before the fix, green after) asserting the built request URL never contains //users when AdminEndpoint has a trailing slash
  • go test ./management/server/idp/... passes (full package, no regressions)
  • golangci-lint run ./management/server/idp/... clean
  • gofmt clean

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Fixed an issue where admin API requests could be built with an extra slash when the server URL ended in /.
    • Improved URL handling so management calls use a normalized base path and avoid malformed requests.
  • Tests
    • Added coverage for admin endpoint URLs with trailing slashes to prevent regressions.
**Original Pull Request:** https://github.com/netbirdio/netbird/pull/6669 **State:** open **Merged:** No --- ## Summary - A trailing slash in the configured Keycloak `AdminEndpoint` produced a double slash in admin API request paths, e.g. `https://<host>/admin/realms/<realm>//users/count?`. - Keycloak >= 26.4.3 requires normalised admin API request paths and rejects the double slash with a 400, which breaks dashboard login and VPN client authentication entirely (see #4979). - Root cause: `management/server/idp/keycloak.go` builds request URLs with `fmt.Sprintf("%s/%s", km.adminEndpoint, resource)` in two call sites (`get`, `DeleteUser`); if the configured endpoint already ends in `/`, this yields `//`. - Fix: normalize once at construction time in `NewKeycloakManager` (`strings.TrimRight(config.AdminEndpoint, "/")`) instead of patching every call site individually. Fixes #4979. ## Test plan - [x] Added `TestKeycloakManager_AdminEndpointTrailingSlash` (red before the fix, green after) asserting the built request URL never contains `//users` when `AdminEndpoint` has a trailing slash - [x] `go test ./management/server/idp/...` passes (full package, no regressions) - [x] `golangci-lint run ./management/server/idp/...` clean - [x] `gofmt` clean 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Fixed an issue where admin API requests could be built with an extra slash when the server URL ended in `/`. * Improved URL handling so management calls use a normalized base path and avoid malformed requests. * **Tests** * Added coverage for admin endpoint URLs with trailing slashes to prevent regressions. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
saavagebueno added the pull-request label 2026-08-05 08:08:58 -04:00
Sign in to join this conversation.
No Label pull-request
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: DYNR/netbird#29789