Skip to content
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
4 changes: 2 additions & 2 deletions lib/authz/permissions.go
Original file line number Diff line number Diff line change
Expand Up @@ -394,8 +394,8 @@ func (a *authorizer) authorizeAdminAction(ctx context.Context, authContext *Cont
return trace.Wrap(err)
}

// Admin actions do not require MFA when MFA is not enabled.
if authpref.GetSecondFactor() == constants.SecondFactorOff {
// Admin actions do not require MFA when Webauthn is not enabled.
if authpref.GetPreferredLocalMFA() != constants.SecondFactorWebauthn {
return nil
}

Expand Down
7 changes: 5 additions & 2 deletions lib/authz/permissions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -446,10 +446,13 @@ func TestAuthorizer_AuthorizeAdminAction(t *testing.T) {
ctx := context.Background()
client, watcher, _ := newTestResources(t)

// Enable OTP.
// Enable Webauthn.
authPreference, err := types.NewAuthPreference(types.AuthPreferenceSpecV2{
Type: constants.Local,
SecondFactor: constants.SecondFactorOTP,
SecondFactor: constants.SecondFactorWebauthn,
Webauthn: &types.Webauthn{
RPID: "localhost",
},
})
require.NoError(t, err)
require.NoError(t, client.SetAuthPreference(ctx, authPreference))
Expand Down
28 changes: 15 additions & 13 deletions rfd/0131-adminitrative-actions-mfa.md
Original file line number Diff line number Diff line change
Expand Up @@ -146,14 +146,10 @@ Connect, Teleport Web UI, Plugins and plugin guides).

### Server configuration

MFA for admin actions will be required on any cluster with
`cluster_auth_preference.second_factor` set to `on`, `optional`, `otp`, or
`webauthn`.

Note: `second_factor: optional` is used to bridge the gap between `off` and
`on`, since existing users will not have an MFA device registered yet. This
change will make it a requirement for admins with `second_factor: optional`
to register an MFA device before they can perform more admin actions.
MFA for admin actions will required on any cluster where Webauthn is the
preferred MFA method. This includes clusters with `cluster_auth_preference.second_factor`
set to `on`, `optional`, or `webauthn` where `cluster_auth_preference.webauthn`
is also set.

#### Built-in Roles

Expand Down Expand Up @@ -439,11 +435,6 @@ Clients will not attempt to provide MFA for [HTTP endpoints](#http-endpoints).
This means that http endpoints converted to gRPC admin action endpoints should
not be made to require MFA until a full major version cycle has passed.

#### TOTP support

The WebUI and Teleport Connect will not support TOTP MFA for admin actions due
to the lack of a universal TOTP MFA prompt for these applications.

### Audit Events

#### Admin Action Events
Expand Down Expand Up @@ -512,3 +503,14 @@ action you can take in Teleport.
However, Hardware Key support is still somewhat limited due to the limitations
of PIV. Most notably, Hardware Key support is not supported on the Web UI, which
is essential for this feature.

#### TOTP

The WebUI and Teleport Connect do not currently have a universal TOTP MFA
prompt, making it difficult to universally support TOTP for admin actions.

If in the future TOTP prompts are added for these applications, we can consider
widening this feature to also cover all clusters where `cluster_auth_preference.second_factor`
is not set to `off`, rather than the more limited scope described in
[Server configuration](#server-configuration). However, this most likely won't
be a priority as we are actually looking to [deprecate TOTP](https://github.com/gravitational/teleport/issues/20725).