Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use second_factors for logic instead of deprecated second_factor. #47426

Merged
merged 10 commits into from
Oct 16, 2024
15 changes: 2 additions & 13 deletions api/types/authentication.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,8 @@ type AuthPreference interface {
// SetType sets the type of authentication: local, saml, or oidc.
SetType(string)

// GetSecondFactor gets the type of second factor.
GetSecondFactor() constants.SecondFactorType
// SetSecondFactor sets the type of second factor.
// Deprecated: only used in tests to set the deprecated off/optional values.
Joerger marked this conversation as resolved.
Show resolved Hide resolved
SetSecondFactor(constants.SecondFactorType)
// GetSecondFactors gets a list of supported second factors.
GetSecondFactors() []SecondFactorType
Expand Down Expand Up @@ -321,16 +320,6 @@ func (c *AuthPreferenceV2) SetType(s string) {
c.Spec.Type = s
}

// GetSecondFactor returns the type of second factor.
func (c *AuthPreferenceV2) GetSecondFactor() constants.SecondFactorType {
// SecondFactors takes priority if set.
if len(c.Spec.SecondFactors) > 0 {
return legacySecondFactorFromSecondFactors(c.Spec.SecondFactors)
}

return c.Spec.SecondFactor
}

// SetSecondFactor sets the type of second factor.
func (c *AuthPreferenceV2) SetSecondFactor(s constants.SecondFactorType) {
c.Spec.SecondFactor = s
Expand Down Expand Up @@ -863,7 +852,7 @@ func (c *AuthPreferenceV2) CheckAndSetDefaults() error {

// String represents a human readable version of authentication settings.
func (c *AuthPreferenceV2) String() string {
return fmt.Sprintf("AuthPreference(Type=%q,SecondFactor=%q)", c.Spec.Type, c.GetSecondFactor())
return fmt.Sprintf("AuthPreference(Type=%q,SecondFactors=%q)", c.Spec.Type, c.GetSecondFactors())
}

// Clone returns a copy of the AuthPreference resource.
Expand Down
17 changes: 6 additions & 11 deletions api/types/authentication_authpreference_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,18 +153,13 @@ func TestAuthPreferenceV2_CheckAndSetDefaults_secondFactor(t *testing.T) {
},
},
assertFn: func(t *testing.T, got *types.AuthPreferenceV2) {
// Did we derive the WebAuthn config from U2F?
if sf := got.GetSecondFactor(); sf == constants.SecondFactorWebauthn ||
sf == constants.SecondFactorOn ||
sf == constants.SecondFactorOptional {
wantWeb := &types.Webauthn{
RPID: "example.com",
AttestationAllowedCAs: []string{yubicoU2FCA},
}
gotWeb, err := got.GetWebauthn()
require.NoError(t, err, "webauthn config not found")
require.Empty(t, cmp.Diff(wantWeb, gotWeb))
wantWeb := &types.Webauthn{
RPID: "example.com",
AttestationAllowedCAs: []string{yubicoU2FCA},
}
gotWeb, err := got.GetWebauthn()
require.NoError(t, err, "webauthn config not found")
require.Empty(t, cmp.Diff(wantWeb, gotWeb))
},
},
{
Expand Down
49 changes: 28 additions & 21 deletions api/types/second_factor.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,30 +25,13 @@ import (
"github.com/gravitational/teleport/api/constants"
)

// legacySecondFactorFromSecondFactors returns a suitable legacy second factor for the given list of second factors.
func legacySecondFactorFromSecondFactors(secondFactors []SecondFactorType) constants.SecondFactorType {
hasOTP := slices.Contains(secondFactors, SecondFactorType_SECOND_FACTOR_TYPE_OTP)
hasWebAuthn := slices.Contains(secondFactors, SecondFactorType_SECOND_FACTOR_TYPE_WEBAUTHN)

switch {
case hasOTP && hasWebAuthn:
return constants.SecondFactorOn
case hasWebAuthn:
return constants.SecondFactorWebauthn
case hasOTP:
return constants.SecondFactorOTP
default:
return constants.SecondFactorOff
}
}

// secondFactorsFromLegacySecondFactor returns the list of SecondFactorTypes supported by the given second factor type.
func secondFactorsFromLegacySecondFactor(sf constants.SecondFactorType) []SecondFactorType {
switch sf {
case constants.SecondFactorOff:
return nil
case constants.SecondFactorOptional, constants.SecondFactorOn:
return []SecondFactorType{SecondFactorType_SECOND_FACTOR_TYPE_WEBAUTHN, SecondFactorType_SECOND_FACTOR_TYPE_OTP}
return []SecondFactorType{SecondFactorType_SECOND_FACTOR_TYPE_OTP, SecondFactorType_SECOND_FACTOR_TYPE_WEBAUTHN}
Joerger marked this conversation as resolved.
Show resolved Hide resolved
case constants.SecondFactorOTP:
return []SecondFactorType{SecondFactorType_SECOND_FACTOR_TYPE_OTP}
case constants.SecondFactorWebauthn:
Expand All @@ -58,9 +41,32 @@ func secondFactorsFromLegacySecondFactor(sf constants.SecondFactorType) []Second
}
}

// LegacySecondFactorFromSecondFactors returns a suitable legacy second factor for the given list of second factors.
func LegacySecondFactorFromSecondFactors(secondFactors []SecondFactorType) constants.SecondFactorType {
hasOTP := slices.Contains(secondFactors, SecondFactorType_SECOND_FACTOR_TYPE_OTP)
hasWebAuthn := slices.Contains(secondFactors, SecondFactorType_SECOND_FACTOR_TYPE_WEBAUTHN)
Joerger marked this conversation as resolved.
Show resolved Hide resolved
hasSSO := slices.Contains(secondFactors, SecondFactorType_SECOND_FACTOR_TYPE_SSO)

switch {
case hasOTP && hasWebAuthn:
return constants.SecondFactorOn
case hasWebAuthn:
return constants.SecondFactorWebauthn
case hasOTP:
return constants.SecondFactorOTP
case hasSSO:
// In the WebUI, we can treat exclusive SSO MFA as disabled. In practice this means
// things like the "add MFA device" button is disabled, but SSO MFA prompts will still work.
// TODO(Joerger): Ensure that SSO MFA flows work in the WebUI with this change, once implemented.
return constants.SecondFactorOff
default:
return constants.SecondFactorOff
}
}

// MarshalJSON marshals SecondFactorType to string.
func (s *SecondFactorType) MarshalYAML() (interface{}, error) {
val, err := s.encode()
val, err := s.Encode()
if err != nil {
return nil, trace.Wrap(err)
}
Expand All @@ -81,7 +87,7 @@ func (s *SecondFactorType) UnmarshalYAML(unmarshal func(interface{}) error) erro

// MarshalJSON marshals SecondFactorType to string.
func (s *SecondFactorType) MarshalJSON() ([]byte, error) {
val, err := s.encode()
val, err := s.Encode()
if err != nil {
return nil, trace.Wrap(err)
}
Expand Down Expand Up @@ -110,7 +116,8 @@ const (
secondFactorTypeSSOString = "sso"
)

func (s *SecondFactorType) encode() (string, error) {
// Encode encodes the SecondFactorType in string form.
func (s *SecondFactorType) Encode() (string, error) {
switch *s {
case SecondFactorType_SECOND_FACTOR_TYPE_UNSPECIFIED:
return "", nil
Expand Down
48 changes: 44 additions & 4 deletions api/types/second_factor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func TestEncodeDecodeSecondFactorType(t *testing.T) {
} {
t.Run(tt.secondFactorType.String(), func(t *testing.T) {
t.Run("encode", func(t *testing.T) {
encoded, err := tt.secondFactorType.encode()
encoded, err := tt.secondFactorType.Encode()
assert.NoError(t, err)
assert.Equal(t, tt.encoded, encoded)
})
Expand All @@ -57,7 +57,7 @@ func TestEncodeDecodeSecondFactorType(t *testing.T) {
}
}

func TestLegacySecondFactorsFromLegacySecondFactor(t *testing.T) {
func TestSecondFactorsFromLegacySecondFactor(t *testing.T) {
for _, tt := range []struct {
sf constants.SecondFactorType
sfs []SecondFactorType
Expand All @@ -72,11 +72,11 @@ func TestLegacySecondFactorsFromLegacySecondFactor(t *testing.T) {
},
{
sf: constants.SecondFactorOptional,
sfs: []SecondFactorType{SecondFactorType_SECOND_FACTOR_TYPE_WEBAUTHN, SecondFactorType_SECOND_FACTOR_TYPE_OTP},
sfs: []SecondFactorType{SecondFactorType_SECOND_FACTOR_TYPE_OTP, SecondFactorType_SECOND_FACTOR_TYPE_WEBAUTHN},
},
{
sf: constants.SecondFactorOn,
sfs: []SecondFactorType{SecondFactorType_SECOND_FACTOR_TYPE_WEBAUTHN, SecondFactorType_SECOND_FACTOR_TYPE_OTP},
sfs: []SecondFactorType{SecondFactorType_SECOND_FACTOR_TYPE_OTP, SecondFactorType_SECOND_FACTOR_TYPE_WEBAUTHN},
},
{
sf: constants.SecondFactorOTP,
Expand All @@ -92,3 +92,43 @@ func TestLegacySecondFactorsFromLegacySecondFactor(t *testing.T) {
})
}
}

func TestLegacySecondFactorFromSecondFactors(t *testing.T) {
for _, tt := range []struct {
sfs []SecondFactorType
sf constants.SecondFactorType
}{
{
sfs: nil,
sf: constants.SecondFactorOff,
},
{
sfs: []SecondFactorType{},
sf: constants.SecondFactorOff,
},
{
sfs: []SecondFactorType{SecondFactorType_SECOND_FACTOR_TYPE_OTP},
sf: constants.SecondFactorOTP,
},
{
sfs: []SecondFactorType{SecondFactorType_SECOND_FACTOR_TYPE_WEBAUTHN},
sf: constants.SecondFactorWebauthn,
},
{
sfs: []SecondFactorType{SecondFactorType_SECOND_FACTOR_TYPE_SSO},
sf: constants.SecondFactorOff,
},
{
sfs: []SecondFactorType{
SecondFactorType_SECOND_FACTOR_TYPE_OTP,
SecondFactorType_SECOND_FACTOR_TYPE_WEBAUTHN,
SecondFactorType_SECOND_FACTOR_TYPE_SSO,
},
sf: constants.SecondFactorOn,
},
} {
t.Run(string(tt.sfs), func(t *testing.T) {
assert.Equal(t, tt.sf, LegacySecondFactorFromSecondFactors(tt.sfs))
})
}
}
2 changes: 1 addition & 1 deletion integration/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8823,7 +8823,7 @@ func testModeratedSessions(t *testing.T, suite *integrationTestSuite) {
// Enable web service.
cfg := suite.defaultServiceConfig()
cfg.Auth.Enabled = true
cfg.Auth.Preference.SetSecondFactor("on")
cfg.Auth.Preference.SetSecondFactors(types.SecondFactorType_SECOND_FACTOR_TYPE_WEBAUTHN)
cfg.Auth.Preference.(*types.AuthPreferenceV2).Spec.RequireMFAType = types.RequireMFAType_SESSION
cfg.Auth.Preference.SetWebauthn(&types.Webauthn{RPID: "127.0.0.1"})
cfg.Proxy.DisableWebService = false
Expand Down
Loading
Loading