mirror of
https://github.com/netbirdio/netbird.git
synced 2026-08-15 12:11:28 +02:00
Compare commits
1 Commits
main
...
fix/pkce-f
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
0738734b6e |
@@ -199,7 +199,15 @@ type loginHintSetter interface {
|
||||
}
|
||||
|
||||
func (a *Auth) foregroundGetTokenInfo(authClient *auth.Auth, urlOpener URLOpener, isAndroidTV bool) (*auth.TokenInfo, error) {
|
||||
oAuthFlow, err := authClient.GetOAuthFlow(a.ctx, isAndroidTV)
|
||||
return a.foregroundGetTokenInfoFlow(authClient, urlOpener, isAndroidTV, false)
|
||||
}
|
||||
|
||||
// foregroundGetTokenInfoFlow runs the interactive flow. sessionExtend tells the
|
||||
// server the token will renew this peer's session rather than log a peer in, so
|
||||
// it can rule out a silent authorization the IdP could answer from an unrelated
|
||||
// account. See PKCEAuthorizationFlowRequest.
|
||||
func (a *Auth) foregroundGetTokenInfoFlow(authClient *auth.Auth, urlOpener URLOpener, isAndroidTV bool, sessionExtend bool) (*auth.TokenInfo, error) {
|
||||
oAuthFlow, err := authClient.GetOAuthFlow(a.ctx, isAndroidTV, sessionExtend)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to get OAuth flow: %v", err)
|
||||
}
|
||||
|
||||
@@ -293,11 +293,13 @@ func (c *Client) extendAuthSession(ctx context.Context, urlOpener URLOpener, isA
|
||||
}
|
||||
defer authClient.Close()
|
||||
|
||||
// Passing the config path makes the flow pick up the login_hint: an extend
|
||||
// renews the session of the account already signed in, so it must not stop to
|
||||
// offer a choice.
|
||||
// Passing the config path makes the flow pick up the login_hint. That alone
|
||||
// cannot keep the IdP on this profile's account though — a hint is only a
|
||||
// suggestion, and a silent authorization is answered from whatever session the
|
||||
// IdP already has, which need not be this peer's when several accounts are
|
||||
// signed in. Marking the flow as an extend lets the server rule that out.
|
||||
a := NewAuthWithConfig(ctx, cfg, cfgPath)
|
||||
tokenInfo, err := a.foregroundGetTokenInfo(authClient, urlOpener, isAndroidTV)
|
||||
tokenInfo, err := a.foregroundGetTokenInfoFlow(authClient, urlOpener, isAndroidTV, true)
|
||||
if err != nil {
|
||||
return fmt.Errorf("interactive sso login failed: %v", err)
|
||||
}
|
||||
|
||||
@@ -408,7 +408,7 @@ func foregroundGetTokenInfo(ctx context.Context, cmd *cobra.Command, config *pro
|
||||
hint = profileState.Email
|
||||
}
|
||||
|
||||
oAuthFlow, err := auth.NewOAuthFlow(ctx, config, util.HasGraphicalSession(), false, hint)
|
||||
oAuthFlow, err := auth.NewOAuthFlow(ctx, config, util.HasGraphicalSession(), false, hint, false)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
@@ -103,7 +103,7 @@ func (a *Auth) IsSSOSupported(ctx context.Context) (bool, error) {
|
||||
|
||||
err := a.withRetry(ctx, func(client *mgm.GrpcClient) error {
|
||||
// Try PKCE flow first
|
||||
_, err := a.getPKCEFlow(client)
|
||||
_, err := a.getPKCEFlow(client, false)
|
||||
if err == nil {
|
||||
supportsSSO = true
|
||||
return nil
|
||||
@@ -138,7 +138,11 @@ func (a *Auth) IsSSOSupported(ctx context.Context) (bool, error) {
|
||||
|
||||
// GetOAuthFlow returns an OAuth flow (PKCE or Device) using the existing management connection
|
||||
// This avoids creating a new connection to the management server
|
||||
func (a *Auth) GetOAuthFlow(ctx context.Context, forceDeviceAuth bool) (OAuthFlow, error) {
|
||||
//
|
||||
// sessionExtend marks the flow as renewing an existing peer's session rather than
|
||||
// logging one in; the server needs it to rule out a silent authorization that the
|
||||
// IdP could answer from another account. See PKCEAuthorizationFlowRequest.
|
||||
func (a *Auth) GetOAuthFlow(ctx context.Context, forceDeviceAuth bool, sessionExtend bool) (OAuthFlow, error) {
|
||||
var flow OAuthFlow
|
||||
var err error
|
||||
|
||||
@@ -149,7 +153,7 @@ func (a *Auth) GetOAuthFlow(ctx context.Context, forceDeviceAuth bool) (OAuthFlo
|
||||
}
|
||||
|
||||
// Try PKCE flow first
|
||||
flow, err = a.getPKCEFlow(client)
|
||||
flow, err = a.getPKCEFlow(client, sessionExtend)
|
||||
if err != nil {
|
||||
// If PKCE not supported, try Device flow
|
||||
if s, ok := status.FromError(err); ok && (s.Code() == codes.NotFound || s.Code() == codes.Unimplemented) {
|
||||
@@ -229,8 +233,8 @@ func (a *Auth) Login(ctx context.Context, setupKey string, jwtToken string) (err
|
||||
}
|
||||
|
||||
// getPKCEFlow retrieves PKCE authorization flow configuration and creates a flow instance
|
||||
func (a *Auth) getPKCEFlow(client *mgm.GrpcClient) (*PKCEAuthorizationFlow, error) {
|
||||
protoFlow, err := client.GetPKCEAuthorizationFlow()
|
||||
func (a *Auth) getPKCEFlow(client *mgm.GrpcClient, sessionExtend bool) (*PKCEAuthorizationFlow, error) {
|
||||
protoFlow, err := client.GetPKCEAuthorizationFlow(sessionExtend)
|
||||
if err != nil {
|
||||
if s, ok := status.FromError(err); ok && s.Code() == codes.NotFound {
|
||||
log.Warnf("server couldn't find pkce flow, contact admin: %v", err)
|
||||
|
||||
@@ -70,12 +70,15 @@ func shouldUseDeviceFlow(force bool, isUnixDesktopClient bool) bool {
|
||||
//
|
||||
// On Linux distros without desktop environment support, it only tries to initialize the Device Code Flow
|
||||
// forceDeviceCodeFlow can be used to skip PKCE and go directly to Device Code Flow (e.g., for Android TV)
|
||||
func NewOAuthFlow(ctx context.Context, config *profilemanager.Config, isUnixDesktopClient bool, forceDeviceCodeFlow bool, hint string) (OAuthFlow, error) {
|
||||
//
|
||||
// sessionExtend marks the flow as renewing an existing peer's session rather than
|
||||
// logging one in; see PKCEAuthorizationFlowRequest for what the server makes of it.
|
||||
func NewOAuthFlow(ctx context.Context, config *profilemanager.Config, isUnixDesktopClient bool, forceDeviceCodeFlow bool, hint string, sessionExtend bool) (OAuthFlow, error) {
|
||||
if shouldUseDeviceFlow(forceDeviceCodeFlow, isUnixDesktopClient) {
|
||||
return authenticateWithDeviceCodeFlow(ctx, config, hint)
|
||||
}
|
||||
|
||||
pkceFlow, err := authenticateWithPKCEFlow(ctx, config, hint)
|
||||
pkceFlow, err := authenticateWithPKCEFlow(ctx, config, hint, sessionExtend)
|
||||
if err != nil {
|
||||
log.Debugf("failed to initialize pkce authentication with error: %v\n", err)
|
||||
log.Debug("falling back to device code flow")
|
||||
@@ -85,14 +88,14 @@ func NewOAuthFlow(ctx context.Context, config *profilemanager.Config, isUnixDesk
|
||||
}
|
||||
|
||||
// authenticateWithPKCEFlow initializes the Proof Key for Code Exchange flow auth flow
|
||||
func authenticateWithPKCEFlow(ctx context.Context, config *profilemanager.Config, hint string) (OAuthFlow, error) {
|
||||
func authenticateWithPKCEFlow(ctx context.Context, config *profilemanager.Config, hint string, sessionExtend bool) (OAuthFlow, error) {
|
||||
authClient, err := NewAuth(ctx, config.PrivateKey, config.ManagementURL, config)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to create auth client: %v", err)
|
||||
}
|
||||
defer authClient.Close()
|
||||
|
||||
pkceFlowInfo, err := authClient.getPKCEFlow(authClient.client)
|
||||
pkceFlowInfo, err := authClient.getPKCEFlow(authClient.client, sessionExtend)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("getting pkce authorization flow info failed with error: %v", err)
|
||||
}
|
||||
|
||||
@@ -87,10 +87,9 @@ func (pm *ProfileManager) SetActiveProfileState(state *ProfileState) error {
|
||||
|
||||
// RemoveProfileState deletes the per-profile state file (which holds the
|
||||
// account email used for the SSO login hint and the UI display). Called after
|
||||
// profile removal; logout keeps the file so the next login can pass the email
|
||||
// as the login_hint. The state file only stores the email, so deleting it is
|
||||
// equivalent to clearing it; the next SSO login recreates it. A missing file
|
||||
// is not an error.
|
||||
// a successful logout so a logged-out profile no longer shows a stale account
|
||||
// email. The state file only stores the email, so deleting it is equivalent to
|
||||
// clearing it; the next SSO login recreates it. A missing file is not an error.
|
||||
func (pm *ProfileManager) RemoveProfileState(profileName string) error {
|
||||
configDir, err := getConfigDir()
|
||||
if err != nil {
|
||||
|
||||
@@ -1,82 +0,0 @@
|
||||
//go:build windows
|
||||
|
||||
package systemops
|
||||
|
||||
import (
|
||||
"math"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
||||
func TestSortRouteCandidates(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
candidates []candidateRoute
|
||||
wantOrder []uint32
|
||||
}{
|
||||
{
|
||||
name: "longest prefix wins over metrics",
|
||||
candidates: []candidateRoute{
|
||||
{interfaceIndex: 1, prefixLength: 0, routeMetric: 0, interfaceMetric: 5},
|
||||
{interfaceIndex: 2, prefixLength: 24, routeMetric: 100, interfaceMetric: 50},
|
||||
},
|
||||
wantOrder: []uint32{2, 1},
|
||||
},
|
||||
{
|
||||
// Windows ranks equal-length prefixes by route metric + interface metric,
|
||||
// so a higher route metric on a low metric interface can still win.
|
||||
name: "combined metric beats route metric alone",
|
||||
candidates: []candidateRoute{
|
||||
{interfaceIndex: 8, prefixLength: 0, routeMetric: 0, interfaceMetric: 100},
|
||||
{interfaceIndex: 5, prefixLength: 0, routeMetric: 10, interfaceMetric: 5},
|
||||
},
|
||||
wantOrder: []uint32{5, 8},
|
||||
},
|
||||
{
|
||||
name: "lower combined metric wins",
|
||||
candidates: []candidateRoute{
|
||||
{interfaceIndex: 5, prefixLength: 0, routeMetric: 300, interfaceMetric: 5},
|
||||
{interfaceIndex: 8, prefixLength: 0, routeMetric: 0, interfaceMetric: 100},
|
||||
},
|
||||
wantOrder: []uint32{8, 5},
|
||||
},
|
||||
{
|
||||
name: "equal combined metric falls back to route metric",
|
||||
candidates: []candidateRoute{
|
||||
{interfaceIndex: 1, prefixLength: 0, routeMetric: 20, interfaceMetric: 10},
|
||||
{interfaceIndex: 2, prefixLength: 0, routeMetric: 5, interfaceMetric: 25},
|
||||
},
|
||||
wantOrder: []uint32{2, 1},
|
||||
},
|
||||
{
|
||||
// The metrics are uint32 on the Windows side, so the sum must not wrap.
|
||||
name: "combined metric beyond the uint32 range",
|
||||
candidates: []candidateRoute{
|
||||
{interfaceIndex: 1, prefixLength: 0, routeMetric: math.MaxUint32, interfaceMetric: 5},
|
||||
{interfaceIndex: 2, prefixLength: 0, routeMetric: math.MaxUint32 - 10, interfaceMetric: 5},
|
||||
},
|
||||
wantOrder: []uint32{2, 1},
|
||||
},
|
||||
{
|
||||
name: "unknown interface metric ranks on route metric only",
|
||||
candidates: []candidateRoute{
|
||||
{interfaceIndex: 1, prefixLength: 0, routeMetric: 30, interfaceMetric: -1},
|
||||
{interfaceIndex: 2, prefixLength: 0, routeMetric: 5, interfaceMetric: 10},
|
||||
},
|
||||
wantOrder: []uint32{2, 1},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
sortRouteCandidates(tt.candidates)
|
||||
|
||||
got := make([]uint32, 0, len(tt.candidates))
|
||||
for _, c := range tt.candidates {
|
||||
got = append(got, c.interfaceIndex)
|
||||
}
|
||||
assert.Equal(t, tt.wantOrder, got)
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -882,40 +882,26 @@ func getInterfaceMetric(interfaceIndex uint32, family int16) int {
|
||||
return int(ipInterfaceRow.Metric)
|
||||
}
|
||||
|
||||
// sortRouteCandidates sorts route candidates by priority: prefix length -> combined metric -> route metric.
|
||||
// Windows prefers the longest matching prefix and, among prefixes of the same length, the lowest metric, see
|
||||
// https://learn.microsoft.com/en-us/windows-hardware/customize/desktop/unattend/microsoft-windows-tcpip-interfaces-interface-routes-route-metric
|
||||
// sortRouteCandidates sorts route candidates by priority: prefix length -> route metric -> interface metric
|
||||
func sortRouteCandidates(candidates []candidateRoute) {
|
||||
sort.Slice(candidates, func(i, j int) bool {
|
||||
if candidates[i].prefixLength != candidates[j].prefixLength {
|
||||
return candidates[i].prefixLength > candidates[j].prefixLength
|
||||
}
|
||||
mi, mj := combinedMetric(candidates[i]), combinedMetric(candidates[j])
|
||||
if mi != mj {
|
||||
return mi < mj
|
||||
if candidates[i].routeMetric != candidates[j].routeMetric {
|
||||
return candidates[i].routeMetric < candidates[j].routeMetric
|
||||
}
|
||||
return candidates[i].routeMetric < candidates[j].routeMetric
|
||||
return candidates[i].interfaceMetric < candidates[j].interfaceMetric
|
||||
})
|
||||
}
|
||||
|
||||
// combinedMetric returns the effective metric Windows uses to rank routes with an equal prefix length:
|
||||
// the sum of the route metric and the metric of the interface the route is on, see
|
||||
// https://learn.microsoft.com/en-us/windows-server/networking/technologies/network-subsystem/net-sub-interface-metric
|
||||
// An unknown interface metric contributes nothing.
|
||||
func combinedMetric(candidate candidateRoute) uint64 {
|
||||
if candidate.interfaceMetric < 0 {
|
||||
return uint64(candidate.routeMetric)
|
||||
}
|
||||
return uint64(candidate.routeMetric) + uint64(candidate.interfaceMetric)
|
||||
}
|
||||
|
||||
// GetBestInterface finds the best interface for reaching a destination,
|
||||
// excluding the VPN interface to avoid routing loops.
|
||||
//
|
||||
// Route selection priority:
|
||||
// 1. Longest prefix match (most specific route)
|
||||
// 2. Lowest combined metric (route metric + interface metric)
|
||||
// 3. Lowest route metric.
|
||||
// 2. Lowest route metric
|
||||
// 3. Lowest interface metric
|
||||
func GetBestInterface(dest netip.Addr, vpnIntf string) (*net.Interface, error) {
|
||||
var skipInterfaceIndex int
|
||||
if vpnIntf != "" {
|
||||
@@ -939,6 +925,7 @@ func GetBestInterface(dest netip.Addr, vpnIntf string) (*net.Interface, error) {
|
||||
return nil, fmt.Errorf("no route to %s", dest)
|
||||
}
|
||||
|
||||
// Sort routes: prefix length -> route metric -> interface metric
|
||||
sortRouteCandidates(candidates)
|
||||
|
||||
for _, candidate := range candidates {
|
||||
|
||||
@@ -5,7 +5,6 @@ package systemops
|
||||
import (
|
||||
"errors"
|
||||
"net"
|
||||
"net/netip"
|
||||
"syscall"
|
||||
"testing"
|
||||
|
||||
@@ -30,7 +29,6 @@ func ensureIPv6DefaultRoute(t *testing.T) {
|
||||
}
|
||||
if err := netlink.RouteAdd(route); err != nil {
|
||||
if errors.Is(err, syscall.EEXIST) {
|
||||
requireUsableIPv6Nexthop(t)
|
||||
return
|
||||
}
|
||||
t.Skipf("install IPv6 fallback default route: %v", err)
|
||||
@@ -40,36 +38,4 @@ func ensureIPv6DefaultRoute(t *testing.T) {
|
||||
t.Logf("delete IPv6 fallback default route: %v", err)
|
||||
}
|
||||
})
|
||||
|
||||
requireUsableIPv6Nexthop(t)
|
||||
}
|
||||
|
||||
// requireUsableIPv6Nexthop skips the test unless the resolved IPv6 default
|
||||
// nexthop can actually carry a route. Installing the default route succeeding
|
||||
// does not imply the kernel accepts it as a nexthop for a concrete prefix.
|
||||
func requireUsableIPv6Nexthop(t *testing.T) {
|
||||
t.Helper()
|
||||
|
||||
nexthop, err := GetNextHop(netip.IPv6Unspecified())
|
||||
if err != nil {
|
||||
t.Skipf("resolve IPv6 default nexthop: %v", err)
|
||||
}
|
||||
|
||||
probe := &netlink.Route{
|
||||
Scope: netlink.SCOPE_UNIVERSE,
|
||||
Table: syscall.RT_TABLE_MAIN,
|
||||
Family: netlink.FAMILY_V6,
|
||||
Dst: &net.IPNet{IP: net.ParseIP("100::64"), Mask: net.CIDRMask(128, 128)},
|
||||
}
|
||||
require.NoError(t, addNextHop(nexthop, probe), "build IPv6 probe route")
|
||||
|
||||
switch err := netlink.RouteAdd(probe); {
|
||||
case err == nil:
|
||||
if err := netlink.RouteDel(probe); err != nil && !errors.Is(err, syscall.ESRCH) {
|
||||
t.Logf("delete IPv6 probe route: %v", err)
|
||||
}
|
||||
case errors.Is(err, syscall.EEXIST):
|
||||
default:
|
||||
t.Skipf("IPv6 nexthop %s unusable for route installation: %v", nexthop, err)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -429,7 +429,7 @@ func (c *Client) LoginForMobile() string {
|
||||
return fmt.Sprintf("failed to load config: %v", err)
|
||||
}
|
||||
|
||||
oAuthFlow, err := auth.NewOAuthFlow(ctx, cfg, false, false, "")
|
||||
oAuthFlow, err := auth.NewOAuthFlow(ctx, cfg, false, false, "", false)
|
||||
if err != nil {
|
||||
return err.Error()
|
||||
}
|
||||
|
||||
@@ -323,7 +323,7 @@ func (a *Auth) login(urlOpener URLOpener, forceDeviceAuth bool, deviceName strin
|
||||
const authInfoRequestTimeout = 30 * time.Second
|
||||
|
||||
func (a *Auth) foregroundGetTokenInfo(authClient *auth.Auth, urlOpener URLOpener, forceDeviceAuth bool) (*auth.TokenInfo, error) {
|
||||
oAuthFlow, err := authClient.GetOAuthFlow(a.ctx, forceDeviceAuth)
|
||||
oAuthFlow, err := authClient.GetOAuthFlow(a.ctx, forceDeviceAuth, false)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to get OAuth flow: %v", err)
|
||||
}
|
||||
|
||||
@@ -679,7 +679,7 @@ func (s *Server) Login(callerCtx context.Context, msg *proto.LoginRequest) (*pro
|
||||
if msg.Hint != nil {
|
||||
hint = *msg.Hint
|
||||
}
|
||||
oAuthFlow, err := auth.NewOAuthFlow(ctx, config, msg.IsUnixDesktopClient, false, hint)
|
||||
oAuthFlow, err := auth.NewOAuthFlow(ctx, config, msg.IsUnixDesktopClient, false, hint, false)
|
||||
if err != nil {
|
||||
state.Set(internal.StatusLoginFailed)
|
||||
return nil, err
|
||||
@@ -1724,7 +1724,7 @@ func (s *Server) RequestJWTAuth(
|
||||
}
|
||||
|
||||
// the daemon has no graphical session of its own, only the caller can answer this
|
||||
oAuthFlow, err := auth.NewOAuthFlow(ctx, config, msg.GetHasGraphicalSession(), false, hint)
|
||||
oAuthFlow, err := auth.NewOAuthFlow(ctx, config, msg.GetHasGraphicalSession(), false, hint, false)
|
||||
if err != nil {
|
||||
return nil, gstatus.Errorf(codes.Internal, "failed to create OAuth flow: %v", err)
|
||||
}
|
||||
@@ -1828,7 +1828,7 @@ func (s *Server) RequestExtendAuthSession(
|
||||
}
|
||||
|
||||
// the daemon has no graphical session of its own, only the caller can answer this
|
||||
oAuthFlow, err := auth.NewOAuthFlow(ctx, config, msg.GetHasGraphicalSession(), false, hint)
|
||||
oAuthFlow, err := auth.NewOAuthFlow(ctx, config, msg.GetHasGraphicalSession(), false, hint, true)
|
||||
if err != nil {
|
||||
return nil, gstatus.Errorf(codes.Internal, "failed to create OAuth flow: %v", err)
|
||||
}
|
||||
|
||||
@@ -6,11 +6,9 @@ import (
|
||||
"context"
|
||||
"time"
|
||||
|
||||
log "github.com/sirupsen/logrus"
|
||||
"google.golang.org/grpc/codes"
|
||||
gstatus "google.golang.org/grpc/status"
|
||||
|
||||
"github.com/netbirdio/netbird/client/internal/profilemanager"
|
||||
"github.com/netbirdio/netbird/client/proto"
|
||||
)
|
||||
|
||||
@@ -62,19 +60,9 @@ func (s *Session) RequestExtend(ctx context.Context, p ExtendStartParams) (Exten
|
||||
|
||||
// a request from the UI implies a graphical session, which the daemon cannot detect itself
|
||||
req := &proto.RequestExtendAuthSessionRequest{HasGraphicalSession: true}
|
||||
hint := p.Hint
|
||||
if hint == "" {
|
||||
pm := profilemanager.NewProfileManager()
|
||||
if active, perr := pm.GetActiveProfile(); perr != nil {
|
||||
log.Debugf("failed to get active profile for login hint: %v", perr)
|
||||
} else if state, serr := pm.GetProfileState(active.ID); serr != nil {
|
||||
log.Debugf("failed to get profile state for login hint: %v", serr)
|
||||
} else {
|
||||
hint = state.Email
|
||||
}
|
||||
}
|
||||
if hint != "" {
|
||||
req.Hint = &hint
|
||||
if p.Hint != "" {
|
||||
h := p.Hint
|
||||
req.Hint = &h
|
||||
}
|
||||
|
||||
resp, err := cli.RequestExtendAuthSession(ctx, req)
|
||||
|
||||
@@ -123,16 +123,8 @@ func (s *Connection) Login(ctx context.Context, p LoginParams) (LoginResult, err
|
||||
if p.PreSharedKey != "" {
|
||||
req.OptionalPreSharedKey = ptrStr(p.PreSharedKey)
|
||||
}
|
||||
hint := p.Hint
|
||||
if hint == "" && profileID != "" {
|
||||
if state, serr := profilemanager.NewProfileManager().GetProfileState(profilemanager.ID(profileID)); serr == nil {
|
||||
hint = state.Email
|
||||
} else {
|
||||
log.Debugf("failed to get profile state for login hint: %v", serr)
|
||||
}
|
||||
}
|
||||
if hint != "" {
|
||||
req.Hint = ptrStr(hint)
|
||||
if p.Hint != "" {
|
||||
req.Hint = ptrStr(p.Hint)
|
||||
}
|
||||
|
||||
resp, err := cli.Login(ctx, req)
|
||||
@@ -236,6 +228,16 @@ func (s *Connection) Logout(ctx context.Context, p LogoutParams) error {
|
||||
return s.classifyDaemonError(err)
|
||||
}
|
||||
|
||||
// The daemon runs as root and can't reach the user-owned per-profile state
|
||||
// file holding the account email (see Profiles.List), so clear the stale
|
||||
// email here; the next SSO login recreates it.
|
||||
if p.ProfileName != "" {
|
||||
if err := profilemanager.NewProfileManager().RemoveProfileState(p.ProfileName); err != nil {
|
||||
// Non-fatal: the logout itself succeeded.
|
||||
log.Warnf("failed to remove profile state for %s: %v", p.ProfileName, err)
|
||||
}
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -259,7 +261,7 @@ func (s *Connection) waitSSOLogin(ctx context.Context, p WaitSSOParams) (string,
|
||||
|
||||
// Persist the account email the same way the CLI does after its own
|
||||
// WaitSSOLogin: the daemon returns it but cannot store it, since it runs as
|
||||
// root and the per-profile state file is user-owned (see Profiles.List).
|
||||
// root and the per-profile state file is user-owned (see Logout below).
|
||||
// Without this the profile has no email, so Profiles.List shows no account
|
||||
// and later logins and session extends go out without a login_hint —
|
||||
// leaving the IdP to guess which account was meant.
|
||||
|
||||
@@ -162,9 +162,8 @@ func (s *Profiles) Remove(ctx context.Context, p ProfileRef) error {
|
||||
}
|
||||
|
||||
// The daemon deletes what it owns but runs as root, so it leaves the
|
||||
// user-owned state file holding the account email behind. Logout keeps the
|
||||
// email on purpose so later logins can pass it as the login_hint; profile
|
||||
// removal is what deletes it. Legacy profiles are keyed by name rather than by a
|
||||
// user-owned state file holding the account email behind (same split as
|
||||
// Connection.Logout). Legacy profiles are keyed by name rather than by a
|
||||
// generated ID, so a recreated profile of the same name would inherit the
|
||||
// deleted one's email and offer it as the login_hint.
|
||||
//
|
||||
|
||||
75
management/internals/shared/grpc/pkce_flow_test.go
Normal file
75
management/internals/shared/grpc/pkce_flow_test.go
Normal file
@@ -0,0 +1,75 @@
|
||||
package grpc
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
|
||||
"github.com/netbirdio/netbird/shared/management/client/common"
|
||||
"github.com/netbirdio/netbird/shared/management/proto"
|
||||
)
|
||||
|
||||
func TestApplySessionExtendFlowPolicy(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
flow *proto.PKCEAuthorizationFlow
|
||||
sessionExtend bool
|
||||
disablePromptLogin bool
|
||||
loginFlag uint32
|
||||
}{
|
||||
{
|
||||
name: "extend forces prompt=login over a silent flow",
|
||||
flow: &proto.PKCEAuthorizationFlow{
|
||||
ProviderConfig: &proto.ProviderConfig{
|
||||
DisablePromptLogin: true,
|
||||
LoginFlag: uint32(common.LoginFlagMaxAge0),
|
||||
},
|
||||
},
|
||||
sessionExtend: true,
|
||||
disablePromptLogin: false,
|
||||
loginFlag: uint32(common.LoginFlagPromptLogin),
|
||||
},
|
||||
{
|
||||
name: "extend replaces max_age=0 so login_hint is honoured",
|
||||
flow: &proto.PKCEAuthorizationFlow{
|
||||
ProviderConfig: &proto.ProviderConfig{
|
||||
DisablePromptLogin: false,
|
||||
LoginFlag: uint32(common.LoginFlagMaxAge0),
|
||||
},
|
||||
},
|
||||
sessionExtend: true,
|
||||
disablePromptLogin: false,
|
||||
loginFlag: uint32(common.LoginFlagPromptLogin),
|
||||
},
|
||||
{
|
||||
name: "login keeps the configured flow untouched",
|
||||
flow: &proto.PKCEAuthorizationFlow{
|
||||
ProviderConfig: &proto.ProviderConfig{
|
||||
DisablePromptLogin: true,
|
||||
LoginFlag: uint32(common.LoginFlagMaxAge0),
|
||||
},
|
||||
},
|
||||
sessionExtend: false,
|
||||
disablePromptLogin: true,
|
||||
loginFlag: uint32(common.LoginFlagMaxAge0),
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range tests {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
applySessionExtendFlowPolicy(tc.flow, tc.sessionExtend)
|
||||
cfg := tc.flow.GetProviderConfig()
|
||||
assert.Equal(t, tc.disablePromptLogin, cfg.GetDisablePromptLogin())
|
||||
assert.Equal(t, tc.loginFlag, cfg.GetLoginFlag())
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// A provider config is not guaranteed to be present on the response; clearing
|
||||
// the flag must not panic when the validator returned an empty flow.
|
||||
func TestApplySessionExtendFlowPolicyWithoutProviderConfig(t *testing.T) {
|
||||
assert.NotPanics(t, func() {
|
||||
applySessionExtendFlowPolicy(&proto.PKCEAuthorizationFlow{}, true)
|
||||
applySessionExtendFlowPolicy(nil, true)
|
||||
})
|
||||
}
|
||||
@@ -1180,7 +1180,8 @@ func (s *Server) GetPKCEAuthorizationFlow(ctx context.Context, req *proto.Encryp
|
||||
return nil, status.Errorf(codes.Internal, "failed to get server key")
|
||||
}
|
||||
|
||||
err = encryption.DecryptMessage(peerKey, key, req.Body, &proto.PKCEAuthorizationFlowRequest{})
|
||||
flowReq := &proto.PKCEAuthorizationFlowRequest{}
|
||||
err = encryption.DecryptMessage(peerKey, key, req.Body, flowReq)
|
||||
if err != nil {
|
||||
errMSG := fmt.Sprintf("error while decrypting peer's message with Wireguard public key %s.", req.WgPubKey)
|
||||
log.WithContext(ctx).Warn(errMSG)
|
||||
@@ -1224,6 +1225,7 @@ func (s *Server) GetPKCEAuthorizationFlow(ctx context.Context, req *proto.Encryp
|
||||
}
|
||||
|
||||
flowInfoResp := s.integratedPeerValidator.ValidateFlowResponse(ctx, peerKey.String(), initInfoFlow)
|
||||
applySessionExtendFlowPolicy(flowInfoResp, flowReq.GetSessionExtend())
|
||||
|
||||
encryptedResp, err := encryption.EncryptMessage(peerKey, key, flowInfoResp)
|
||||
if err != nil {
|
||||
@@ -1236,6 +1238,32 @@ func (s *Server) GetPKCEAuthorizationFlow(ctx context.Context, req *proto.Encryp
|
||||
}, nil
|
||||
}
|
||||
|
||||
// applySessionExtendFlowPolicy forces a prompt=login flow for a session extend.
|
||||
//
|
||||
// An extend renews the session of one specific peer, so its token has to come
|
||||
// from the account that peer is registered under. A flow that does not prompt
|
||||
// leaves the choice to the IdP, which answers a silent authorization from any
|
||||
// session it already holds — not necessarily this peer's account when several
|
||||
// are signed in, and login_hint is a suggestion the IdP may ignore. The token
|
||||
// then fails the jwt.UserID == peer.UserID check in ExtendAuthSession, and the
|
||||
// user is given no opportunity to pick a different account.
|
||||
//
|
||||
// LoginFlagPromptLogin rather than max_age=0: both re-authenticate, but with
|
||||
// prompt=login the IdP honours login_hint and offers the peer's own account,
|
||||
// whereas max_age=0 leaves the user to find it among every account signed in.
|
||||
//
|
||||
// Called after ValidateFlowResponse so that a per-peer override cannot reinstate
|
||||
// the silent flow for an extend.
|
||||
func applySessionExtendFlowPolicy(flow *proto.PKCEAuthorizationFlow, sessionExtend bool) {
|
||||
if !sessionExtend {
|
||||
return
|
||||
}
|
||||
if cfg := flow.GetProviderConfig(); cfg != nil {
|
||||
cfg.DisablePromptLogin = false
|
||||
cfg.LoginFlag = uint32(common.LoginFlagPromptLogin)
|
||||
}
|
||||
}
|
||||
|
||||
// SyncMeta endpoint is used to synchronize peer's system metadata and notifies the connected,
|
||||
// peer's under the same account of any updates.
|
||||
func (s *Server) SyncMeta(ctx context.Context, req *proto.EncryptedMessage) (*proto.Empty, error) {
|
||||
|
||||
@@ -21,7 +21,7 @@ type Client interface {
|
||||
// is not eligible for session extension.
|
||||
ExtendAuthSession(sysInfo *system.Info, jwtToken string) (*proto.ExtendAuthSessionResponse, error)
|
||||
GetDeviceAuthorizationFlow() (*proto.DeviceAuthorizationFlow, error)
|
||||
GetPKCEAuthorizationFlow() (*proto.PKCEAuthorizationFlow, error)
|
||||
GetPKCEAuthorizationFlow(sessionExtend bool) (*proto.PKCEAuthorizationFlow, error)
|
||||
GetServerURL() string
|
||||
// IsHealthy returns the current connection status without blocking.
|
||||
// Used by the engine to monitor connectivity in the background.
|
||||
|
||||
@@ -595,7 +595,12 @@ func Test_GetPKCEAuthorizationFlow(t *testing.T) {
|
||||
},
|
||||
}
|
||||
|
||||
var gotRequest mgmtProto.PKCEAuthorizationFlowRequest
|
||||
mgmtMockServer.GetPKCEAuthorizationFlowFunc = func(ctx context.Context, req *mgmtProto.EncryptedMessage) (*mgmtProto.EncryptedMessage, error) {
|
||||
if err := encryption.DecryptMessage(client.key.PublicKey(), serverKey, req.Body, &gotRequest); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
encryptedResp, err := encryption.EncryptMessage(client.key.PublicKey(), serverKey, expectedFlowInfo)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
@@ -608,11 +613,13 @@ func Test_GetPKCEAuthorizationFlow(t *testing.T) {
|
||||
}, nil
|
||||
}
|
||||
|
||||
flowInfo, err := client.GetPKCEAuthorizationFlow()
|
||||
flowInfo, err := client.GetPKCEAuthorizationFlow(true)
|
||||
if err != nil {
|
||||
t.Error("error while retrieving pkce auth flow information")
|
||||
}
|
||||
|
||||
assert.True(t, gotRequest.GetSessionExtend(), "session extend should reach the server")
|
||||
|
||||
assert.Equal(t, expectedFlowInfo.ProviderConfig.ClientID, flowInfo.ProviderConfig.ClientID, "provider configured client ID should match")
|
||||
assert.Equal(t, expectedFlowInfo.ProviderConfig.ClientSecret, flowInfo.ProviderConfig.ClientSecret, "provider configured client secret should match") //nolint:staticcheck
|
||||
}
|
||||
|
||||
@@ -701,7 +701,11 @@ func (c *GrpcClient) GetDeviceAuthorizationFlow() (*proto.DeviceAuthorizationFlo
|
||||
|
||||
// GetPKCEAuthorizationFlow returns a pkce authorization flow information.
|
||||
// It also takes care of encrypting and decrypting messages.
|
||||
func (c *GrpcClient) GetPKCEAuthorizationFlow() (*proto.PKCEAuthorizationFlow, error) {
|
||||
//
|
||||
// sessionExtend tells the server the flow will renew an existing peer's session
|
||||
// rather than log one in, so it can rule out a configuration that would let the
|
||||
// IdP answer from an unrelated account. See PKCEAuthorizationFlowRequest.
|
||||
func (c *GrpcClient) GetPKCEAuthorizationFlow(sessionExtend bool) (*proto.PKCEAuthorizationFlow, error) {
|
||||
if !c.ready() {
|
||||
return nil, fmt.Errorf("no connection to management in order to get pkce authorization flow")
|
||||
}
|
||||
@@ -714,7 +718,7 @@ func (c *GrpcClient) GetPKCEAuthorizationFlow() (*proto.PKCEAuthorizationFlow, e
|
||||
mgmCtx, cancel := context.WithTimeout(c.ctx, time.Second*2)
|
||||
defer cancel()
|
||||
|
||||
message := &proto.PKCEAuthorizationFlowRequest{}
|
||||
message := &proto.PKCEAuthorizationFlowRequest{SessionExtend: sessionExtend}
|
||||
encryptedMSG, err := encryption.EncryptMessage(*serverKey, c.key, message)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
|
||||
@@ -16,7 +16,7 @@ type MockClient struct {
|
||||
LoginFunc func(info *system.Info, sshKey []byte, dnsLabels domain.List) (*proto.LoginResponse, error)
|
||||
ExtendAuthSessionFunc func(info *system.Info, jwtToken string) (*proto.ExtendAuthSessionResponse, error)
|
||||
GetDeviceAuthorizationFlowFunc func() (*proto.DeviceAuthorizationFlow, error)
|
||||
GetPKCEAuthorizationFlowFunc func() (*proto.PKCEAuthorizationFlow, error)
|
||||
GetPKCEAuthorizationFlowFunc func(sessionExtend bool) (*proto.PKCEAuthorizationFlow, error)
|
||||
GetServerURLFunc func() string
|
||||
HealthCheckFunc func() error
|
||||
SyncMetaFunc func(sysInfo *system.Info) error
|
||||
@@ -80,11 +80,11 @@ func (m *MockClient) GetDeviceAuthorizationFlow() (*proto.DeviceAuthorizationFlo
|
||||
return m.GetDeviceAuthorizationFlowFunc()
|
||||
}
|
||||
|
||||
func (m *MockClient) GetPKCEAuthorizationFlow() (*proto.PKCEAuthorizationFlow, error) {
|
||||
func (m *MockClient) GetPKCEAuthorizationFlow(sessionExtend bool) (*proto.PKCEAuthorizationFlow, error) {
|
||||
if m.GetPKCEAuthorizationFlowFunc == nil {
|
||||
return nil, nil
|
||||
}
|
||||
return m.GetPKCEAuthorizationFlowFunc()
|
||||
return m.GetPKCEAuthorizationFlowFunc(sessionExtend)
|
||||
}
|
||||
|
||||
func (m *MockClient) HealthCheck() error {
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
@@ -530,8 +530,18 @@ message DeviceAuthorizationFlow {
|
||||
}
|
||||
}
|
||||
|
||||
// PKCEAuthorizationFlowRequest empty struct for future expansion
|
||||
message PKCEAuthorizationFlowRequest {}
|
||||
// PKCEAuthorizationFlowRequest asks for the PKCE flow configuration to use for
|
||||
// an upcoming authorization request.
|
||||
message PKCEAuthorizationFlowRequest {
|
||||
// SessionExtend indicates the flow will renew the SSO session of a peer that
|
||||
// is already registered, rather than log in or register one. An extend is
|
||||
// bound to the account that peer belongs to, so the server must not answer it
|
||||
// with a configuration that lets the IdP reply from whatever session is
|
||||
// already active: with several accounts signed in at the IdP that need not be
|
||||
// the peer's own, and the resulting token is rejected as a peer/user mismatch
|
||||
// with no way for the user to correct it.
|
||||
bool SessionExtend = 1;
|
||||
}
|
||||
|
||||
// PKCEAuthorizationFlow represents Authorization Code Flow information
|
||||
// that can be used by the client to login initiate a Oauth 2.0 authorization code grant flow
|
||||
|
||||
Reference in New Issue
Block a user