opensource/teleport
3
0
Fork 0
mirror of https://github.com/gravitational/teleport.git synced 2026-09-21 05:55:42 +08:00
Code Issues Packages Projects Releases Wiki Activity

Add leaf cluster to MFA prompt and fix resource casing; Improve look of MFA modal in Connect (#66714)

Browse Source 
* Improve look of ReAuthenticate in Connect

* Display `<Text>` for reason only if reason is present.
* Drop "from trusted cluster" being appended to reason. This will be
  moved to api/mfa/prompt.go.
* Center text for browsermfa and sso so it matches webauthn.
* Consistent comma at the end of text for each method.

* Add leaf cluster name to prompt reason for MFA

* Fix resource type casing in MFA prompt

* Assert MFA prompt in tests
This commit is contained in:
Maja
2026-05-14 07:27:11 +00:00
committed by GitHub
parent 23716c4d8a
commit 212f0acc26
5 changed files with 69 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files
api/mfa/prompt.go
+6 -1
View File
@@ -88,9 +88,14 @@ func WithPromptReasonAdminAction() PromptOpt {
}
// WithPromptReasonSessionMFA sets the prompt's PromptReason field to a standard session mfa message.
func WithPromptReasonSessionMFA(serviceType, serviceName string) PromptOpt {
// If leafClusterName is non-empty, it is included in the message to indicate that the resource
// belongs to a leaf cluster.
func WithPromptReasonSessionMFA(serviceType, serviceName, leafClusterName string) PromptOpt {
return func(cfg *PromptConfig) {
cfg.PromptReason = fmt.Sprintf("MFA is required to access %s %q", serviceType, serviceName)
if leafClusterName != "" {
cfg.PromptReason += fmt.Sprintf(" from leaf cluster %q", leafClusterName)
}
// Set the extensions to scope USER_SESSION, which we know is true, but
// don't override any explicitly-set extensions (as they are likely more
lib/client/cluster_client.go
+12 -6
View File
@@ -495,25 +495,31 @@ func (c *ClusterClient) performSessionMFACeremony(ctx context.Context, rootClien
return nil, trace.Wrap(err)
}
mfaAgainstRoot := c.cluster == rootClient.cluster
var leafClusterName string
if !mfaAgainstRoot {
leafClusterName = c.cluster
}
var promptOpts []mfa.PromptOpt
switch {
case params.NodeName != "":
promptOpts = append(promptOpts, mfa.WithPromptReasonSessionMFA("Node", params.NodeName))
promptOpts = append(promptOpts, mfa.WithPromptReasonSessionMFA("node", params.NodeName, leafClusterName))
case params.KubernetesCluster != "":
promptOpts = append(promptOpts, mfa.WithPromptReasonSessionMFA("Kubernetes cluster", params.KubernetesCluster))
promptOpts = append(promptOpts, mfa.WithPromptReasonSessionMFA("Kubernetes cluster", params.KubernetesCluster, leafClusterName))
case params.RouteToDatabase.ServiceName != "":
promptOpts = append(promptOpts, mfa.WithPromptReasonSessionMFA("Database", params.RouteToDatabase.ServiceName))
promptOpts = append(promptOpts, mfa.WithPromptReasonSessionMFA("database", params.RouteToDatabase.ServiceName, leafClusterName))
case params.RouteToApp.Name != "":
promptOpts = append(promptOpts, mfa.WithPromptReasonSessionMFA("Application", params.RouteToApp.Name))
promptOpts = append(promptOpts, mfa.WithPromptReasonSessionMFA("application", params.RouteToApp.Name, leafClusterName))
case params.RouteToWindowsDesktop.WindowsDesktop != "":
promptOpts = append(promptOpts, mfa.WithPromptReasonSessionMFA("Windows desktop", params.RouteToWindowsDesktop.WindowsDesktop))
promptOpts = append(promptOpts, mfa.WithPromptReasonSessionMFA("Windows desktop", params.RouteToWindowsDesktop.WindowsDesktop, leafClusterName))
}
result, err := PerformSessionMFACeremony(ctx, PerformSessionMFACeremonyParams{
CurrentAuthClient: c.AuthClient,
RootAuthClient: rootClient.AuthClient,
MFACeremony: c.tc.NewMFACeremony(),
MFAAgainstRoot: c.cluster == rootClient.cluster,
MFAAgainstRoot: mfaAgainstRoot,
MFARequiredReq: mfaRequiredReq,
CertsReq: certsReq,
KeyRing: keyRing,
lib/client/cluster_client_test.go
+42 -5
View File
@@ -17,6 +17,7 @@
package client
import (
"cmp"
"context"
"crypto/ecdsa"
"crypto/rsa"
@@ -174,7 +175,12 @@ func TestIssueUserCertsWithMFA(t *testing.T) {
params ReissueParams
prompt fakePrompt
signatureAlgorithmSuite types.SignatureAlgorithmSuite
assertion func(t *testing.T, result *IssueUserCertsWithMFAResult, err error)
// clientCluster overrides the ClusterClient's cluster field. Defaults to "test".
clientCluster string
// wantPromptReason, when non-empty, asserts the PromptReason passed to the
// MFA prompt constructor.
wantPromptReason string
assertion func(t *testing.T, result *IssueUserCertsWithMFAResult, err error)
}{
{
name: "ssh no mfa",
@@ -190,9 +196,10 @@ func TestIssueUserCertsWithMFA(t *testing.T) {
},
},
{
name: "ssh mfa success",
mfaRequired: proto.MFARequired_MFA_REQUIRED_YES,
params: ReissueParams{NodeName: "test"},
name: "ssh mfa success",
mfaRequired: proto.MFARequired_MFA_REQUIRED_YES,
params: ReissueParams{NodeName: "test"},
wantPromptReason: `MFA is required to access node "test"`,
assertion: func(t *testing.T, result *IssueUserCertsWithMFAResult, err error) {
require.NoError(t, err)
require.NotNil(t, result)
@@ -546,6 +553,28 @@ func TestIssueUserCertsWithMFA(t *testing.T) {
require.NotNil(t, result.KeyRing)
},
},
{
name: "session MFA from a leaf cluster mentions the leaf in the prompt reason",
mfaRequired: proto.MFARequired_MFA_REQUIRED_YES,
clientCluster: "leaf",
params: ReissueParams{
NodeName: "test",
// In real world RouteToCluster would be "leaf", but this doesn't work
// with how the test is currently set up.
RouteToCluster: "test",
AuthClient: fakeAuthClient{
isMFARequired: func(ctx context.Context, req *proto.IsMFARequiredRequest) (*proto.IsMFARequiredResponse, error) {
return &proto.IsMFARequiredResponse{MFARequired: proto.MFARequired_MFA_REQUIRED_YES, Required: true}, nil
},
generateUserCerts: defaultGenerateUserCerts,
},
},
wantPromptReason: `MFA is required to access node "test" from leaf cluster "leaf"`,
assertion: func(t *testing.T, result *IssueUserCertsWithMFAResult, err error) {
require.NoError(t, err)
require.NotNil(t, result)
},
},
}
for _, test := range tests {
@@ -563,6 +592,8 @@ func TestIssueUserCertsWithMFA(t *testing.T) {
suite = types.SignatureAlgorithmSuite_SIGNATURE_ALGORITHM_SUITE_BALANCED_V1
}
var capturedPromptCfg *libmfa.PromptConfig
clientCluster := cmp.Or(test.clientCluster, "test")
clt := &ClusterClient{
tc: &TeleportClient{
localAgent: agent,
@@ -572,6 +603,7 @@ func TestIssueUserCertsWithMFA(t *testing.T) {
HostLogin: "default-login",
Tracer: tracing.NoopTracer("test"),
MFAPromptConstructor: func(cfg *libmfa.PromptConfig) mfa.Prompt {
capturedPromptCfg = cfg
return test.prompt
},
Stderr: io.Discard,
@@ -597,7 +629,7 @@ func TestIssueUserCertsWithMFA(t *testing.T) {
generateUserCerts: defaultGenerateUserCerts,
},
Tracer: tracing.NoopTracer("test"),
cluster: "test",
cluster: clientCluster,
root: "test",
}
@@ -605,6 +637,11 @@ func TestIssueUserCertsWithMFA(t *testing.T) {
result, err := clt.IssueUserCertsWithMFA(ctx, test.params)
test.assertion(t, result, err)
if test.wantPromptReason != "" {
require.NotNil(t, capturedPromptCfg, "MFA prompt constructor was not invoked")
require.Equal(t, test.wantPromptReason, capturedPromptCfg.PromptReason)
}
})
}
}
web/packages/teleterm/src/ui/ClusterConnect/ClusterLogin/FormLogin/PromptSsoStatus/PromptSsoStatus.tsx
+2 -2
View File
@@ -28,8 +28,8 @@ export default function PromptSsoStatus(props: {
}) {
const { onCancel, ssoPrompt } = props;
return (
<Flex gap={4} flexDirection="column" alignItems="flex-start">
<Box width="100%" style={{ position: 'relative' }}>
<Flex width="100%" gap={4} flexDirection="column" alignItems="flex-start">
<Box width="100%" textAlign="center" style={{ position: 'relative' }}>
<Text bold mb={2} textAlign="center">
{ssoPrompt === 'follow-browser-steps' && (
<>Please follow the steps in the browser to&nbsp;authenticate.</>
web/packages/teleterm/src/ui/ModalsHost/modals/ReAuthenticate/ReAuthenticate.tsx
+7 -8
View File
@@ -79,8 +79,6 @@ export const ReAuthenticate: FC<{
// In practice though we should not end up in a situation where this modal is shown but the
// cluster does not exist in the app.
rootCluster?.proxyHost || rootClusterName;
const clusterName = routing.parseClusterName(clusterUri);
const isLeafCluster = routing.isLeafCluster(clusterUri);
// maybeOpenBrowser opens the browser if the given mfaType is SSO of Browser MFA
function maybeOpenBrowser(mfaType: AvailableMfaType) {
@@ -190,10 +188,7 @@ export const ReAuthenticate: FC<{
<DialogContent mb={4}>
<Flex flexDirection="column" gap={4} alignItems="flex-start">
<Text>
{req.reason}
{isLeafCluster && ` from trusted cluster "${clusterName}"`}
</Text>
{req.reason && <Text>{req.reason}</Text>}
<Flex width="100%" gap={3} flexWrap="wrap">
{availableMfaTypes.length > 1 && (
@@ -223,7 +218,7 @@ export const ReAuthenticate: FC<{
textAlign="center"
style={{ position: 'relative' }}
>
<Text bold>Insert your security key and tap it</Text>
<Text bold>Insert your security key and tap it.</Text>
<LinearProgress />
</Box>
</>
@@ -234,7 +229,11 @@ export const ReAuthenticate: FC<{
)}
{selectedMfaType.value === 'browsermfa' && (
<Box width="100%" style={{ position: 'relative' }}>
<Box
width="100%"
textAlign="center"
style={{ position: 'relative' }}
>
<Text bold>
Please follow the steps in the browser to authenticate.
</Text>