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

feat: SSO MFA - support for SAML ForceAuthn for login and mfa checks #46703

Merged
merged 5 commits into from
Oct 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions api/proto/teleport/legacy/types/types.proto
Original file line number Diff line number Diff line change
Expand Up @@ -4764,6 +4764,9 @@ message SAMLConnectorSpecV2 {
string SingleLogoutURL = 16 [(gogoproto.jsontag) = "single_logout_url,omitempty"];
// MFASettings contains settings to enable SSO MFA checks through this auth connector.
SAMLConnectorMFASettings MFASettings = 17 [(gogoproto.jsontag) = "mfa,omitempty"];
// ForceAuthn specified whether re-authentication should be forced on login. UNSPECIFIED
// is treated as NO.
SAMLForceAuthn ForceAuthn = 18 [(gogoproto.jsontag) = "force_authn,omitempty"];
}

// SAMLConnectorMFASettings contains SAML MFA settings.
Expand All @@ -4775,6 +4778,21 @@ message SAMLConnectorMFASettings {
string entity_descriptor = 2;
// EntityDescriptorUrl is a URL that supplies a configuration XML.
string entity_descriptor_url = 3;
// ForceAuthn specified whether re-authentication should be forced for MFA checks. UNSPECIFIED is
// treated as YES to always re-authentication for MFA checks. This should only be set to NO if the
// IdP is setup to perform MFA checks on top of active user sessions.
SAMLForceAuthn force_authn = 4;
}

// SAMLForceAuthn specified whether existing SAML sessions should be accepted or re-authentication
// should be forced.
enum SAMLForceAuthn {
// UNSPECIFIED is treated as the default value for the context; NO for login, YES for MFA checks.
FORCE_AUTHN_UNSPECIFIED = 0;
// YES re-authentication should be forced for existing SAML sessions..
FORCE_AUTHN_YES = 1;
// NO re-authentication should not be forced for existing SAML sessions.
FORCE_AUTHN_NO = 2;
}

// SAMLAuthRequest is a request to authenticate with SAML
Expand Down
16 changes: 16 additions & 0 deletions api/types/saml.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,8 @@ type SAMLConnector interface {
IsMFAEnabled() bool
// WithMFASettings returns the connector will some settings overwritten set from MFA settings.
WithMFASettings() error
// GetForceAuthn returns ForceAuthn
GetForceAuthn() bool
}

// NewSAMLConnector returns a new SAMLConnector based off a name and SAMLConnectorSpecV2.
Expand Down Expand Up @@ -420,9 +422,23 @@ func (o *SAMLConnectorV2) WithMFASettings() error {

o.Spec.EntityDescriptor = o.Spec.MFASettings.EntityDescriptor
o.Spec.EntityDescriptorURL = o.Spec.MFASettings.EntityDescriptorUrl

switch o.Spec.MFASettings.ForceAuthn {
case SAMLForceAuthn_FORCE_AUTHN_UNSPECIFIED:
// Default to YES.
o.Spec.ForceAuthn = SAMLForceAuthn_FORCE_AUTHN_YES
default:
o.Spec.ForceAuthn = o.Spec.MFASettings.ForceAuthn
}

return nil
}

// GetForceAuthn returns ForceAuthn
func (o *SAMLConnectorV2) GetForceAuthn() bool {
return o.Spec.ForceAuthn == SAMLForceAuthn_FORCE_AUTHN_YES
}

// setStaticFields sets static resource header and metadata fields.
func (o *SAMLConnectorV2) setStaticFields() {
o.Kind = KindSAMLConnector
Expand Down
59 changes: 52 additions & 7 deletions api/types/saml_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,35 +14,37 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

package types
package types_test

import (
"testing"

"github.com/stretchr/testify/require"

"github.com/gravitational/teleport/api/types"
)

// TestSAMLSecretsStrip tests the WithoutSecrets method on SAMLConnectorV2.
func TestSAMLSecretsStrip(t *testing.T) {
connector, err := NewSAMLConnector("test", SAMLConnectorSpecV2{
connector, err := types.NewSAMLConnector("test", types.SAMLConnectorSpecV2{
AssertionConsumerService: "test",
SSO: "test",
EntityDescriptor: "test",
SigningKeyPair: &AsymmetricKeyPair{PrivateKey: "test"},
EncryptionKeyPair: &AsymmetricKeyPair{PrivateKey: "test"},
SigningKeyPair: &types.AsymmetricKeyPair{PrivateKey: "test"},
EncryptionKeyPair: &types.AsymmetricKeyPair{PrivateKey: "test"},
})
require.NoError(t, err)
require.Equal(t, "test", connector.GetSigningKeyPair().PrivateKey)
require.Equal(t, "test", connector.GetEncryptionKeyPair().PrivateKey)

withoutSecrets := connector.WithoutSecrets().(*SAMLConnectorV2)
withoutSecrets := connector.WithoutSecrets().(*types.SAMLConnectorV2)
require.Empty(t, withoutSecrets.GetSigningKeyPair().PrivateKey)
require.Empty(t, withoutSecrets.GetEncryptionKeyPair().PrivateKey)
}

// TestSAMLAcsUriHasConnector tests that the ACS URI has the connector ID as the last part if IdP-initiated login is enabled.
func TestSAMLACSURIHasConnectorName(t *testing.T) {
connector, err := NewSAMLConnector("myBusinessConnector", SAMLConnectorSpecV2{
connector, err := types.NewSAMLConnector("myBusinessConnector", types.SAMLConnectorSpecV2{
AssertionConsumerService: "https://teleport.local/webapi/v1/saml/acs",
SSO: "test",
EntityDescriptor: "test",
Expand All @@ -52,7 +54,7 @@ func TestSAMLACSURIHasConnectorName(t *testing.T) {
require.Nil(t, connector)
require.Error(t, err)

connector, err = NewSAMLConnector("myBusinessConnector", SAMLConnectorSpecV2{
connector, err = types.NewSAMLConnector("myBusinessConnector", types.SAMLConnectorSpecV2{
AssertionConsumerService: "https://teleport.local/webapi/v1/saml/acs/myBusinessConnector",
SSO: "test",
EntityDescriptor: "test",
Expand All @@ -62,3 +64,46 @@ func TestSAMLACSURIHasConnectorName(t *testing.T) {
require.NotNil(t, connector)
require.NoError(t, err)
}

func TestSAMLForceAuthn(t *testing.T) {
for _, tt := range []struct {
name string
forceAuthn types.SAMLForceAuthn
expectBase bool
expectMFA bool
}{
{
name: "force_authn unspecified",
forceAuthn: types.SAMLForceAuthn_FORCE_AUTHN_UNSPECIFIED,
expectBase: false,
expectMFA: true,
}, {
name: "force_authn yes",
forceAuthn: types.SAMLForceAuthn_FORCE_AUTHN_YES,
expectBase: true,
expectMFA: true,
}, {
name: "force_authn no",
forceAuthn: types.SAMLForceAuthn_FORCE_AUTHN_NO,
expectBase: false,
expectMFA: false,
},
} {
t.Run(tt.name, func(t *testing.T) {
samlConnector := types.SAMLConnectorV2{
Spec: types.SAMLConnectorSpecV2{
ForceAuthn: tt.forceAuthn,
MFASettings: &types.SAMLConnectorMFASettings{
Enabled: true,
ForceAuthn: tt.forceAuthn,
},
},
}

require.Equal(t, tt.expectBase, samlConnector.GetForceAuthn(), "expected force_authn to be %v but got %v", tt.expectBase, samlConnector.GetForceAuthn())

require.NoError(t, samlConnector.WithMFASettings())
require.Equal(t, tt.expectMFA, samlConnector.GetForceAuthn(), "expected force_authn to be %v for mfa but got %v", tt.expectMFA, samlConnector.GetForceAuthn())
})
}
}
Loading
Loading