Skip to content

Commit

Permalink
feat: SSO MFA - WebUI backend implementation (#47832)
Browse files Browse the repository at this point in the history
* Add SSO MFA ceremony support to WebUI per-session MFA.

* Add display name to SSO MFA device; Add SSO MFA device to SSO challenge.

* Camel case json tags for front end.

* Add sso channel ID for front end broadcast channel logic.

* Fix backwards compatiblity issue; fix error message.

* Fix test.

* Fallback to connector id if display name isn't set.

* Fix test.

* Resolve comment.

* Use uuid.NewRandom.

* go mod tidy
  • Loading branch information
Joerger authored Nov 6, 2024
1 parent 63b0037 commit 313acfa
Show file tree
Hide file tree
Showing 27 changed files with 2,271 additions and 1,984 deletions.
2,013 changes: 1,038 additions & 975 deletions api/client/proto/authservice.pb.go

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions api/proto/teleport/legacy/client/proto/authservice.proto
Original file line number Diff line number Diff line change
Expand Up @@ -1253,6 +1253,8 @@ message SSOChallenge {
string request_id = 1;
// RedirectUrl is an IdP redirect URL to initate the SSO MFA flow.
string redirect_url = 2;
// Device is the SSO device corresponding to the challenge.
types.SSOMFADevice device = 3;
}

// SSOResponse is a response to SSOChallenge.
Expand Down
2 changes: 2 additions & 0 deletions api/proto/teleport/legacy/types/types.proto
Original file line number Diff line number Diff line change
Expand Up @@ -3838,6 +3838,8 @@ message SSOMFADevice {
string connector_id = 1;
// connector_type is the type of the SSO connector.
string connector_type = 2;
// display_name is the display name of the SSO connector
string display_name = 3;
}

// WebauthnLocalAuth holds settings necessary for local webauthn use.
Expand Down
1,909 changes: 977 additions & 932 deletions api/types/types.pb.go

Large diffs are not rendered by default.

52 changes: 52 additions & 0 deletions integrations/event-handler/go.sum

Large diffs are not rendered by default.

5 changes: 5 additions & 0 deletions lib/auth/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import (
"github.com/gravitational/teleport/api/utils/keys"
"github.com/gravitational/teleport/lib/auth/authclient"
"github.com/gravitational/teleport/lib/authz"
"github.com/gravitational/teleport/lib/client/sso"
"github.com/gravitational/teleport/lib/defaults"
"github.com/gravitational/teleport/lib/events"
"github.com/gravitational/teleport/lib/loginrule"
Expand Down Expand Up @@ -982,6 +983,10 @@ func ValidateClientRedirect(clientRedirect string, ssoTestFlow bool, settings *t
if err != nil {
return trace.Wrap(err, "parsing client redirect URL")
}
if u.Path == sso.WebMFARedirect {
// If this is a SSO redirect in the WebUI, allow.
return nil
}
if u.Opaque != "" {
return trace.BadParameter("unexpected opaque client redirect URL")
}
Expand Down
1 change: 1 addition & 0 deletions lib/auth/grpcserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,7 @@ func TestMFADeviceManagement_SSO(t *testing.T) {
Sso: &types.SSOMFADevice{
ConnectorId: samlConnector.GetName(),
ConnectorType: samlConnector.GetKind(),
DisplayName: samlConnector.GetDisplay(),
},
})
require.NoError(t, err)
Expand Down
5 changes: 4 additions & 1 deletion lib/auth/sso_mfa.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,10 @@ import (

// beginSSOMFAChallenge creates a new SSO MFA auth request and session data for the given user and sso device.
func (a *Server) beginSSOMFAChallenge(ctx context.Context, user string, sso *types.SSOMFADevice, ssoClientRedirectURL string, ext *mfav1.ChallengeExtensions) (*proto.SSOChallenge, error) {
chal := new(proto.SSOChallenge)
chal := &proto.SSOChallenge{
Device: sso,
}

switch sso.ConnectorType {
case constants.SAML:
resp, err := a.CreateSAMLAuthRequestForMFA(ctx, types.SAMLAuthRequest{
Expand Down
1 change: 1 addition & 0 deletions lib/auth/sso_mfa_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,7 @@ func TestSSOMFAChallenge_Validation(t *testing.T) {
Sso: &types.SSOMFADevice{
ConnectorId: samlConnector.GetName(),
ConnectorType: samlConnector.GetKind(),
DisplayName: samlConnector.GetDisplay(),
},
})
require.NoError(t, err)
Expand Down
6 changes: 3 additions & 3 deletions lib/client/sso/ceremony.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,15 @@ func NewCLICeremony(rd *Redirector, init CeremonyInit) *Ceremony {

// Ceremony is a customizable SSO MFA ceremony.
type MFACeremony struct {
clientCallbackURL string
close func()
ClientCallbackURL string
HandleRedirect func(ctx context.Context, redirectURL string) error
GetCallbackMFAToken func(ctx context.Context) (string, error)
}

// GetClientCallbackURL returns the client callback URL.
func (m *MFACeremony) GetClientCallbackURL() string {
return m.clientCallbackURL
return m.ClientCallbackURL
}

// Run the SSO MFA ceremony.
Expand Down Expand Up @@ -108,8 +108,8 @@ func (m *MFACeremony) Close() {
// The returned MFACeremony takes ownership of the Redirector.
func NewCLIMFACeremony(rd *Redirector) *MFACeremony {
return &MFACeremony{
clientCallbackURL: rd.ClientCallbackURL,
close: rd.Close,
ClientCallbackURL: rd.ClientCallbackURL,
HandleRedirect: rd.OpenRedirect,
GetCallbackMFAToken: func(ctx context.Context) (string, error) {
loginResp, err := rd.WaitForResponse(ctx)
Expand Down
5 changes: 5 additions & 0 deletions lib/client/sso/redirector.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,11 @@ const (

// DefaultLoginURL is the default login page.
DefaultLoginURL = "/web/login"

// WebMFARedirect is the landing page for SSO MFA in the WebUI. The WebUI set up a listener
// on this page in order to capture the SSO MFA response regardless of what page the challenge
// was requested from.
WebMFARedirect = "/web/sso_confirm"
)

// RedirectorConfig is configuration for an sso redirector.
Expand Down
39 changes: 39 additions & 0 deletions lib/client/weblogin.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,14 @@ type MFAChallengeResponse struct {
TOTPCode string `json:"totp_code,omitempty"`
// WebauthnResponse is a response from a webauthn device.
WebauthnResponse *wantypes.CredentialAssertionResponse `json:"webauthn_response,omitempty"`
// SSOResponse is a response from an SSO MFA flow.
SSOResponse *SSOResponse `json:"sso_response"`
}

// SSOResponse is a json compatible [proto.SSOResponse].
type SSOResponse struct {
RequestID string `json:"requestId,omitempty"`
Token string `json:"token,omitempty"`
}

// GetOptionalMFAResponseProtoReq converts response to a type proto.MFAAuthenticateResponse,
Expand Down Expand Up @@ -457,6 +465,37 @@ type MFAAuthenticateChallenge struct {
WebauthnChallenge *wantypes.CredentialAssertion `json:"webauthn_challenge"`
// TOTPChallenge specifies whether TOTP is supported for this user.
TOTPChallenge bool `json:"totp_challenge"`
// SSOChallenge is an SSO MFA challenge.
SSOChallenge *SSOChallenge `json:"sso_challenge"`
}

// SSOChallenge is a json compatible [proto.SSOChallenge].
type SSOChallenge struct {
RequestID string `json:"requestId,omitempty"`
RedirectURL string `json:"redirectUrl,omitempty"`
Device *SSOMFADevice `json:"device"`
// ChannelID is used by the front end to differentiate multiple ongoing SSO
// MFA requests so they don't interfere with each other.
ChannelID string `json:"channelId"`
}

// SSOMFADevice is a json compatible [proto.SSOMFADevice].
type SSOMFADevice struct {
ConnectorID string `json:"connectorId,omitempty"`
ConnectorType string `json:"connectorType,omitempty"`
DisplayName string `json:"displayName,omitempty"`
}

func SSOChallengeFromProto(ssoChal *proto.SSOChallenge) *SSOChallenge {
return &SSOChallenge{
RequestID: ssoChal.RequestId,
RedirectURL: ssoChal.RedirectUrl,
Device: &SSOMFADevice{
ConnectorID: ssoChal.Device.ConnectorId,
ConnectorType: ssoChal.Device.ConnectorType,
DisplayName: ssoChal.Device.DisplayName,
},
}
}

// MFARegisterChallenge is an MFA register challenge sent on new MFA register.
Expand Down
4 changes: 2 additions & 2 deletions lib/defaults/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -707,8 +707,8 @@ const (
// made for an existing file transfer request
WebsocketFileTransferDecision = "t"

// WebsocketWebauthnChallenge is sending a webauthn challenge.
WebsocketWebauthnChallenge = "n"
// WebsocketMFAChallenge is sending an MFA challenge. Only supports WebAuthn and SSO MFA.
WebsocketMFAChallenge = "n"

// WebsocketSessionMetadata is sending the data for a ssh session.
WebsocketSessionMetadata = "s"
Expand Down
1 change: 1 addition & 0 deletions lib/services/local/users.go
Original file line number Diff line number Diff line change
Expand Up @@ -1489,6 +1489,7 @@ func (s *IdentityService) getSSOMFADevice(ctx context.Context, user string) (*ty
Sso: &types.SSOMFADevice{
ConnectorId: cb.Connector.ID,
ConnectorType: cb.Connector.Type,
DisplayName: mfaConnector.GetDisplay(),
},
})
}
Expand Down
27 changes: 16 additions & 11 deletions lib/services/local/users_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -595,33 +595,40 @@ func TestIdentityService_GetMFADevices_SSO(t *testing.T) {
tests := []struct {
name string
connectorRef *types.ConnectorRef
expectSSODevice bool
expectSSODevice *types.SSOMFADevice
}{
{
name: "non-sso user",
connectorRef: nil,
expectSSODevice: false,
expectSSODevice: nil,
}, {
name: "sso user mfa disabled",
connectorRef: &types.ConnectorRef{
Type: "saml",
ID: samlConnectorNoMFA.GetName(),
},
expectSSODevice: false,
}, {
name: "saml user",
connectorRef: &types.ConnectorRef{
Type: "saml",
ID: samlConnector.GetName(),
},
expectSSODevice: true,
expectSSODevice: &types.SSOMFADevice{
ConnectorType: "saml",
ConnectorId: samlConnector.GetName(),
DisplayName: samlConnector.GetDisplay(),
},
}, {
name: "oidc user",
connectorRef: &types.ConnectorRef{
Type: "oidc",
ID: oidcConnector.GetName(),
},
expectSSODevice: true,
expectSSODevice: &types.SSOMFADevice{
ConnectorType: "oidc",
ConnectorId: oidcConnector.GetName(),
DisplayName: oidcConnector.GetDisplay(),
},
},
}
for _, test := range tests {
Expand All @@ -638,15 +645,13 @@ func TestIdentityService_GetMFADevices_SSO(t *testing.T) {
devs, err := identity.GetMFADevices(ctx, "alice", true /* withSecrets */)
require.NoError(t, err)

if !test.expectSSODevice {
if test.expectSSODevice == nil {
assert.Empty(t, devs)
return
}
expectSSODevice, err := types.NewMFADevice(test.connectorRef.ID, test.connectorRef.ID, clock.Now().UTC(), &types.MFADevice_Sso{
Sso: &types.SSOMFADevice{
ConnectorId: test.connectorRef.ID,
ConnectorType: test.connectorRef.Type,
},

expectSSODevice, err := types.NewMFADevice(test.expectSSODevice.ConnectorId, test.expectSSODevice.DisplayName, clock.Now().UTC(), &types.MFADevice_Sso{
Sso: test.expectSSODevice,
})
require.NoError(t, err)
assert.Equal(t, []*types.MFADevice{expectSSODevice}, devs)
Expand Down
8 changes: 4 additions & 4 deletions lib/srv/desktop/tdp/proto.go
Original file line number Diff line number Diff line change
Expand Up @@ -737,10 +737,10 @@ func DecodeMFA(in byteReader) (*MFA, error) {
}
s := string(mt)
switch s {
case defaults.WebsocketWebauthnChallenge:
case defaults.WebsocketMFAChallenge:
default:
return nil, trace.BadParameter(
"got mfa type %v, expected %v (WebAuthn)", mt, defaults.WebsocketWebauthnChallenge)
"got mfa type %v, expected %v (MFAChallenge)", mt, defaults.WebsocketMFAChallenge)
}

var length uint32
Expand Down Expand Up @@ -780,10 +780,10 @@ func DecodeMFAChallenge(in byteReader) (*MFA, error) {
}
s := string(mt)
switch s {
case defaults.WebsocketWebauthnChallenge:
case defaults.WebsocketMFAChallenge:
default:
return nil, trace.BadParameter(
"got mfa type %v, expected %v (WebAuthn)", mt, defaults.WebsocketWebauthnChallenge)
"got mfa type %v, expected %v (MFAChallenge)", mt, defaults.WebsocketMFAChallenge)
}

var length uint32
Expand Down
6 changes: 3 additions & 3 deletions lib/srv/desktop/tdp/proto_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func TestEncodeDecode(t *testing.T) {
}

func FuzzDecode(f *testing.F) {
var corpus = []string{
corpus := []string{
"0",
"\x02",
"\x1b\xff\xff\x800",
Expand Down Expand Up @@ -125,7 +125,7 @@ func TestMFA(t *testing.T) {
c := NewConn(&fakeConn{Buffer: &buff})

mfaWant := &MFA{
Type: defaults.WebsocketWebauthnChallenge[0],
Type: defaults.WebsocketMFAChallenge[0],
MFAAuthenticateChallenge: &client.MFAAuthenticateChallenge{
WebauthnChallenge: &wantypes.CredentialAssertion{
Response: wantypes.PublicKeyCredentialRequestOptions{
Expand Down Expand Up @@ -159,7 +159,7 @@ func TestMFA(t *testing.T) {
require.Equal(t, mfaWant, mfaGot)

respWant := &MFA{
Type: defaults.WebsocketWebauthnChallenge[0],
Type: defaults.WebsocketMFAChallenge[0],
MFAAuthenticateResponse: &authproto.MFAAuthenticateResponse{
Response: &authproto.MFAAuthenticateResponse_Webauthn{
Webauthn: &wanpb.CredentialAssertionResponse{
Expand Down
11 changes: 11 additions & 0 deletions lib/web/apiserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ import (
"github.com/gravitational/teleport/lib/authz"
"github.com/gravitational/teleport/lib/automaticupgrades"
"github.com/gravitational/teleport/lib/client"
"github.com/gravitational/teleport/lib/client/sso"
"github.com/gravitational/teleport/lib/defaults"
dtconfig "github.com/gravitational/teleport/lib/devicetrust/config"
"github.com/gravitational/teleport/lib/events"
Expand Down Expand Up @@ -2260,6 +2261,16 @@ func ConstructSSHResponse(response AuthParams) (*url.URL, error) {

// Extract secret out of the request.
secretKey := u.Query().Get("secret_key")

// We don't use a secret key for WebUI SSO MFA redirects. The request ID itself is
// kept a secret on the front end to minimize the risk of a phishing attack.
if secretKey == "" && u.Path == sso.WebMFARedirect && response.MFAToken != "" {
q := u.Query()
q.Add("response", string(out))
u.RawQuery = q.Encode()
return u, nil
}

if secretKey == "" {
return nil, trace.BadParameter("missing secret_key")
}
Expand Down
14 changes: 5 additions & 9 deletions lib/web/apiserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2182,7 +2182,7 @@ func TestTerminalRequireSessionMFA(t *testing.T) {

envelope := &terminal.Envelope{
Version: defaults.WebsocketVersion,
Type: defaults.WebsocketWebauthnChallenge,
Type: defaults.WebsocketMFAChallenge,
Payload: string(webauthnResBytes),
}
protoBytes, err := proto.Marshal(envelope)
Expand Down Expand Up @@ -2389,7 +2389,7 @@ func handleDesktopMFAWebauthnChallenge(t *testing.T, ws *websocket.Conn, dev *au
})
require.NoError(t, err)
err = tdpConn.WriteMessage(tdp.MFA{
Type: defaults.WebsocketWebauthnChallenge[0],
Type: defaults.WebsocketMFAChallenge[0],
MFAAuthenticateResponse: &authproto.MFAAuthenticateResponse{
Response: &authproto.MFAAuthenticateResponse_Webauthn{
Webauthn: res.GetWebauthn(),
Expand Down Expand Up @@ -3093,7 +3093,6 @@ func TestPingSSHDialTimeout(t *testing.T) {

// Validate the timeout is the default value.
require.Equal(t, cnc.GetSSHDialTimeout(), out.Proxy.SSH.DialTimeout)

}

// TestConstructSSHResponse checks if the secret package uses AES-GCM to
Expand Down Expand Up @@ -4387,7 +4386,6 @@ func TestClusterKubeResourcesGet(t *testing.T) {
require.NoError(t, json.Unmarshal(re.Bytes(), &resp))
require.ElementsMatch(t, tc.expectedResponse, resp.Items)
}

})
}
}
Expand Down Expand Up @@ -4768,7 +4766,6 @@ func TestGetWebConfig_WithEntitlements(t *testing.T) {
assert.NoError(t, err)
diff := cmp.Diff(expectedCfg, cfg)
assert.Empty(t, diff)

}, time.Second*5, time.Millisecond*50)

// use mock client to assert that if ping returns an error, we'll default to
Expand Down Expand Up @@ -4806,7 +4803,6 @@ func TestGetWebConfig_WithEntitlements(t *testing.T) {
assert.NoError(t, err)
diff := cmp.Diff(expectedCfg, cfg)
assert.Empty(t, diff)

}, time.Second*5, time.Millisecond*50)
}

Expand Down Expand Up @@ -10063,7 +10059,7 @@ func TestModeratedSessionWithMFA(t *testing.T) {

envelope := &terminal.Envelope{
Version: defaults.WebsocketVersion,
Type: defaults.WebsocketWebauthnChallenge,
Type: defaults.WebsocketMFAChallenge,
Payload: string(webauthnResBytes),
}
envelopeBytes, err := proto.Marshal(envelope)
Expand Down Expand Up @@ -10094,7 +10090,7 @@ func TestModeratedSessionWithMFA(t *testing.T) {

envelope := &terminal.Envelope{
Version: defaults.WebsocketVersion,
Type: defaults.WebsocketWebauthnChallenge,
Type: defaults.WebsocketMFAChallenge,
Payload: string(webauthnResBytes),
}
envelopeBytes, err := proto.Marshal(envelope)
Expand Down Expand Up @@ -10132,7 +10128,7 @@ func TestModeratedSessionWithMFA(t *testing.T) {

envelope := &terminal.Envelope{
Version: defaults.WebsocketVersion,
Type: defaults.WebsocketWebauthnChallenge,
Type: defaults.WebsocketMFAChallenge,
Payload: string(webauthnResBytes),
}
envelopeBytes, err := proto.Marshal(envelope)
Expand Down
Loading

0 comments on commit 313acfa

Please sign in to comment.