mirror of
https://github.com/netbirdio/netbird.git
synced 2026-10-24 22:39:09 +02:00
[client] Address review comments on PKCE session extend flow
Fail the PKCE authorization flow test on request error instead of continuing into a nil dereference, and make the godoc comments on the touched exported symbols identifier-leading full sentences.
This commit is contained in:
@@ -136,8 +136,8 @@ func (a *Auth) IsSSOSupported(ctx context.Context) (bool, error) {
|
|||||||
return supportsSSO, err
|
return supportsSSO, err
|
||||||
}
|
}
|
||||||
|
|
||||||
// GetOAuthFlow returns an OAuth flow (PKCE or Device) using the existing management connection
|
// GetOAuthFlow returns an OAuth flow (PKCE or Device) using the existing management connection.
|
||||||
// This avoids creating a new connection to the management server
|
// This avoids creating a new connection to the management server.
|
||||||
//
|
//
|
||||||
// sessionExtend marks the flow as renewing an existing peer's session rather than
|
// 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
|
// logging one in; the server needs it to rule out a silent authorization that the
|
||||||
|
|||||||
@@ -88,16 +88,16 @@ func shouldUseDeviceFlow(force bool, isUnixDesktopClient bool) bool {
|
|||||||
return force || (runtime.GOOS == "linux" || runtime.GOOS == "freebsd") && !isUnixDesktopClient
|
return force || (runtime.GOOS == "linux" || runtime.GOOS == "freebsd") && !isUnixDesktopClient
|
||||||
}
|
}
|
||||||
|
|
||||||
// NewOAuthFlow initializes and returns the appropriate OAuth flow based on the management configuration
|
// NewOAuthFlow initializes and returns the appropriate OAuth flow based on the management configuration.
|
||||||
//
|
//
|
||||||
// It starts by initializing the PKCE.If this process fails, it resorts to the Device Code Flow,
|
// It starts by initializing the PKCE.If this process fails, it resorts to the Device Code Flow,
|
||||||
// and if that also fails, the authentication process is deemed unsuccessful
|
// and if that also fails, the authentication process is deemed unsuccessful
|
||||||
//
|
//
|
||||||
// On Linux distros without desktop environment support, it only tries to initialize the Device Code Flow
|
// 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)
|
// forceDeviceCodeFlow can be used to skip PKCE and go directly to Device Code Flow (e.g., for Android TV).
|
||||||
//
|
//
|
||||||
// sessionExtend marks the flow as renewing an existing peer's session rather than
|
// 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.
|
// 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) {
|
func NewOAuthFlow(ctx context.Context, config *profilemanager.Config, isUnixDesktopClient bool, forceDeviceCodeFlow bool, hint string, sessionExtend bool) (OAuthFlow, error) {
|
||||||
if shouldUseDeviceFlow(forceDeviceCodeFlow, isUnixDesktopClient) {
|
if shouldUseDeviceFlow(forceDeviceCodeFlow, isUnixDesktopClient) {
|
||||||
return authenticateWithDeviceCodeFlow(ctx, config, hint)
|
return authenticateWithDeviceCodeFlow(ctx, config, hint)
|
||||||
|
|||||||
@@ -21,6 +21,10 @@ type Client interface {
|
|||||||
// is not eligible for session extension.
|
// is not eligible for session extension.
|
||||||
ExtendAuthSession(sysInfo *system.Info, jwtToken string) (*proto.ExtendAuthSessionResponse, error)
|
ExtendAuthSession(sysInfo *system.Info, jwtToken string) (*proto.ExtendAuthSessionResponse, error)
|
||||||
GetDeviceAuthorizationFlow() (*proto.DeviceAuthorizationFlow, error)
|
GetDeviceAuthorizationFlow() (*proto.DeviceAuthorizationFlow, error)
|
||||||
|
// GetPKCEAuthorizationFlow returns the PKCE authorization flow information.
|
||||||
|
//
|
||||||
|
// sessionExtend marks the flow as renewing an existing peer's session rather
|
||||||
|
// than logging one in. See PKCEAuthorizationFlowRequest.
|
||||||
GetPKCEAuthorizationFlow(sessionExtend bool) (*proto.PKCEAuthorizationFlow, error)
|
GetPKCEAuthorizationFlow(sessionExtend bool) (*proto.PKCEAuthorizationFlow, error)
|
||||||
GetServerURL() string
|
GetServerURL() string
|
||||||
// IsHealthy returns the current connection status without blocking.
|
// IsHealthy returns the current connection status without blocking.
|
||||||
|
|||||||
@@ -614,9 +614,7 @@ func Test_GetPKCEAuthorizationFlow(t *testing.T) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
flowInfo, err := client.GetPKCEAuthorizationFlow(true)
|
flowInfo, err := client.GetPKCEAuthorizationFlow(true)
|
||||||
if err != nil {
|
require.NoError(t, err)
|
||||||
t.Error("error while retrieving pkce auth flow information")
|
|
||||||
}
|
|
||||||
|
|
||||||
assert.True(t, gotRequest.GetSessionExtend(), "session extend should reach the server")
|
assert.True(t, gotRequest.GetSessionExtend(), "session extend should reach the server")
|
||||||
|
|
||||||
|
|||||||
@@ -738,7 +738,7 @@ func (c *GrpcClient) GetDeviceAuthorizationFlow() (*proto.DeviceAuthorizationFlo
|
|||||||
return flowInfoResp, nil
|
return flowInfoResp, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// GetPKCEAuthorizationFlow returns a pkce authorization flow information.
|
// GetPKCEAuthorizationFlow returns the PKCE authorization flow information.
|
||||||
// It also takes care of encrypting and decrypting messages.
|
// It also takes care of encrypting and decrypting messages.
|
||||||
//
|
//
|
||||||
// sessionExtend tells the server the flow will renew an existing peer's session
|
// sessionExtend tells the server the flow will renew an existing peer's session
|
||||||
|
|||||||
@@ -80,6 +80,7 @@ func (m *MockClient) GetDeviceAuthorizationFlow() (*proto.DeviceAuthorizationFlo
|
|||||||
return m.GetDeviceAuthorizationFlowFunc()
|
return m.GetDeviceAuthorizationFlowFunc()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// GetPKCEAuthorizationFlow returns the PKCE authorization flow information.
|
||||||
func (m *MockClient) GetPKCEAuthorizationFlow(sessionExtend bool) (*proto.PKCEAuthorizationFlow, error) {
|
func (m *MockClient) GetPKCEAuthorizationFlow(sessionExtend bool) (*proto.PKCEAuthorizationFlow, error) {
|
||||||
if m.GetPKCEAuthorizationFlowFunc == nil {
|
if m.GetPKCEAuthorizationFlowFunc == nil {
|
||||||
return nil, nil
|
return nil, nil
|
||||||
|
|||||||
Reference in New Issue
Block a user