diff --git a/client/internal/auth/oauth.go b/client/internal/auth/oauth.go index 3208c1ec9..d573798ec 100644 --- a/client/internal/auth/oauth.go +++ b/client/internal/auth/oauth.go @@ -89,6 +89,12 @@ type oauthFlowInit struct { // releases it. Callers that own a long-lived connection return it with a no-op cleanup. type authFactory func(ctx context.Context) (*Auth, func(), error) +// loginHintSetter is implemented by both concrete flows but is deliberately not part of +// OAuthFlow, so callers reach it through a type assertion. +type loginHintSetter interface { + SetLoginHint(hint string) +} + // fallbackFlow wraps the flow that was picked at initialization time with the flows that were // not tried. Whether the IdP actually serves a flow only shows up when the flow is run: an IdP // with the device grant disabled answers the device code request with 404 even though @@ -125,6 +131,20 @@ func (f *fallbackFlow) GetClientID(ctx context.Context) string { return f.current().GetClientID(ctx) } +// SetLoginHint forwards the hint to the active flow and keeps it for a flow a later fallback +// initializes. Callers that set the hint after building the flow reach the concrete flow through +// a type assertion, which the OAuthFlow interface does not carry, so the wrapper has to offer it +// too or the hint is silently dropped. +func (f *fallbackFlow) SetLoginHint(hint string) { + f.mu.Lock() + defer f.mu.Unlock() + + f.hint = hint + if setter, ok := f.active.(loginHintSetter); ok { + setter.SetLoginHint(hint) + } +} + func (f *fallbackFlow) current() OAuthFlow { f.mu.Lock() defer f.mu.Unlock() diff --git a/client/internal/auth/oauth_test.go b/client/internal/auth/oauth_test.go index 35752849f..dc9f4b8ab 100644 --- a/client/internal/auth/oauth_test.go +++ b/client/internal/auth/oauth_test.go @@ -39,6 +39,10 @@ func (s *stubFlow) GetClientID(context.Context) string { return "" } +func (s *stubFlow) SetLoginHint(hint string) { + s.hint = hint +} + // stubInit returns a flow initializer that yields a named stub flow, or err when err is non-nil. func stubInit(name string, err error) oauthFlowInit { return stubInitFlow(name, err, nil) @@ -222,6 +226,31 @@ func TestFallbackFlowRequestAuthInfo(t *testing.T) { }) } +// TestFallbackFlowSetLoginHint covers the Android SDK's pattern: it sets the login hint after the +// flow is built, through a type assertion that the wrapper must satisfy. +func TestFallbackFlowSetLoginHint(t *testing.T) { + mgmURL, err := url.Parse("https://api.netbird.io:443") + require.NoError(t, err) + a := &Auth{mgmURL: mgmURL} + + idpRejects := fmt.Errorf("%w: request device code returned status 404", errFlowNotConfigured) + flows := []oauthFlowInit{stubInitFlow("device", nil, idpRejects), stubInit("pkce", nil)} + + flow, err := oauthFlowWithFallback(a, nil, flows, "", stubAuthFactory(a)) + require.NoError(t, err) + + setter, ok := flow.(loginHintSetter) + require.True(t, ok, "the wrapper must accept a login hint like the concrete flows do") + setter.SetLoginHint("user@example.com") + assert.Equal(t, "user@example.com", activeStub(t, flow).hint, "the active flow must get the hint") + + // the device flow is rejected by the IdP here, so the hint has to survive into the fallback + _, err = flow.RequestAuthInfo(context.Background()) + require.NoError(t, err) + assert.Equal(t, "pkce", activeStub(t, flow).name) + assert.Equal(t, "user@example.com", activeStub(t, flow).hint, "the fallback flow must get the hint too") +} + func TestWithSetupKeyAdvice(t *testing.T) { other := errors.New("connection refused") assert.Equal(t, other, WithSetupKeyAdvice(other), "only an SSO-unavailable error gets advice")