[client] Unexport GetServerPublicKey, add HealthCheck method

[pappz]

Describe your changes

Simplify the Client interface without modifying any underlying logic. The encryption/decryption behavior is identical, key retrieval is just moved inside the methods that need it.

Fix test encryption to use correct key pairs (client public key as remotePubKey instead of server private key).

• Unexport GetServerPublicKey — callers no longer pass the server key manually; each method fetches it internally
• Remove serverKey parameter from Login, Register, GetDeviceAuthorizationFlow, GetPKCEAuthorizationFlow
• Add HealthCheck() error for active management server connectivity probing

Issue ticket number and link

Stack

Checklist

• Is it a bug fix
• Is a typo/documentation fix
• Is a feature enhancement
• It is a refactor
• Created tests that fail without the change (if possible)

By submitting this pull request, you confirm that you have read and agree to the terms of the Contributor License Agreement.

Documentation

Select exactly one:

• I added/updated documentation for this change
• Documentation is not needed for this change (explain why)

Docs PR URL (required if "docs added" is checked)

Paste the PR link from https://github.com/netbirdio/docs here:

https://github.com/netbirdio/docs/pull/__

Summary by CodeRabbit

• New Features

  • Added an active HealthCheck to verify management service reachability.

• Bug Fixes

  • Made login, registration, and SSO flows more reliable by removing fragile external key retrieval paths.

• Refactor

  • Simplified management client interactions: server key handling is internalized and redundant parameters were removed for cleaner, more maintainable behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 816bafc3-1ed5-4f90-b752-a157ec5042b8

📥 Commits

Reviewing files that changed from the base of the PR and between d562923 and 991ed55.

📒 Files selected for processing (1)

• client/internal/auth/auth.go

🚧 Files skipped from review as they are similar to previous changes (1)

• client/internal/auth/auth.go

---

📝 Walkthrough

Walkthrough

This PR centralizes management-server public key handling inside the management client API: callers no longer fetch or pass the server key. Signatures for Login/Register and authorization-flow RPCs were simplified, HealthCheck/IsHealthy were added, and call sites/tests/mocks were updated accordingly.

Changes

Cohort / File(s)	Summary
Management Client Interface shared/management/client/client.go	Removed GetServerPublicKey() and serverKey parameters from Register, Login, GetDeviceAuthorizationFlow, GetPKCEAuthorizationFlow. Added HealthCheck() error and IsHealthy() bool.
Management Client Implementation shared/management/client/grpc.go	Replaced exported GetServerPublicKey() with internal getServerPublicKey(); updated exported methods to fetch server key internally; added HealthCheck(); adjusted encryption/decryption to use fetched key.
Management Client Mock & Tests shared/management/client/mock.go, shared/management/client/client_test.go	Removed mock/getServerPublicKey usage; updated mock function fields and tests to new method signatures; added HealthCheckFunc and HealthCheck test adjustments.
Auth & Connect Logic client/internal/auth/auth.go, client/internal/connect.go	Removed external server-key fetches; doMgmLogin return signature simplified to error; login/register flows call management client methods without passing server key; peer-registration gating simplified.
Engine Tests & Config client/internal/engine_test.go, client/internal/profilemanager/config.go	Removed GetServerPublicKey() uses; replaced connectivity checks with HealthCheck(); minor test formatting changes and adjusted Register/Login calls.

Sequence Diagram(s)

sequenceDiagram
    participant App as Client/Auth
    participant Mgmt as ManagementClient (GrpcClient)
    participant Server as Management Server

    App->>Mgmt: Login(sysInfo, pubSSHKey, dnsLabels)
    note right of Mgmt: GrpcClient.Login (exported)\nfetches server key internally
    Mgmt->>Mgmt: getServerPublicKey()
    Mgmt->>Server: GetPublicKey (gRPC)
    Server-->>Mgmt: serverPublicKey
    Mgmt->>Mgmt: encrypt login request with serverPublicKey
    Mgmt->>Server: Login(encryptedRequest)
    Server-->>Mgmt: LoginResponse (encrypted)
    Mgmt->>Mgmt: decrypt LoginResponse
    Mgmt-->>App: LoginResponse / error

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• [client] Consolidate authentication logic #5010 — Overlapping changes to auth flow and management client usage patterns (similar removal/centralization of server public key handling).
• [client,management] Rewrite the SSH feature #4015 — Related refactor of management-client signatures and login/register flows tied to server key handling.

Suggested reviewers

• pascal-fischer

Poem

🐰
I hopped through code to tidy the trail,
No more keys passed like a flustered mail.
Server secrets tucked in client lair,
Simpler hops and lighter care. 🥕✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main refactoring changes: unexporting GetServerPublicKey and adding HealthCheck method.
Description check	✅ Passed	The description covers the key changes, marks the PR as a refactor, explains why documentation isn't needed, and completes the required checklist sections.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch refactor/unexport-getserverpublickey-add-healthcheck

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud