Skip to content

Commit

Permalink
NAS-133899 / 25.10 / Reduce number of 2FA public endpoints (#15533)
Browse files Browse the repository at this point in the history
This commit removes several 2FA-related public endpoints from the
API as they are unused by the UI team and are of dubious benefit.

The API response for resetting a user's 2FA token now includes
the token in the response. This simplifies the task for API
users to generate a 2FA token for a user and presenting it for
use by removing the necessity to call `auth.me` after generating
the token.
  • Loading branch information
anodos325 authored Jan 30, 2025
1 parent 07bb4ae commit ac1f80b
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 8 deletions.
12 changes: 9 additions & 3 deletions src/middlewared/middlewared/api/v25_04_0/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -251,14 +251,18 @@ class UserTwofactorConfigArgs(BaseModel):
username: str


@single_argument_result
class UserTwofactorConfigResult(BaseModel):
class UserTwofactorConfigEntry(BaseModel):
provisioning_uri: str | None
secret_configured: bool
interval: int
otp_digits: int


@single_argument_result
class UserTwofactorConfigResult(UserTwofactorConfigEntry):
pass


class UserVerifyTwofactorTokenArgs(BaseModel):
username: str
token: Secret[str | None] = None
Expand Down Expand Up @@ -288,4 +292,6 @@ class UserRenew2faSecretArgs(BaseModel):
twofactor_options: TwofactorOptions


UserRenew2faSecretResult = single_argument_result(UserEntry, "UserRenew2faSecretResult")
@single_argument_result
class UserRenew2faSecretResult(UserEntry):
twofactor_config: UserTwofactorConfigEntry
14 changes: 10 additions & 4 deletions src/middlewared/middlewared/plugins/account_/2fa.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,13 @@ async def provisioning_uri_internal(self, username, user_twofactor_config):
'iXsystems'
)

@api_method(UserProvisioningUriArgs, UserProvisioningUriResult, roles=['ACCOUNT_WRITE'])
@api_method(UserProvisioningUriArgs, UserProvisioningUriResult, private=True)
async def provisioning_uri(self, username):
"""
Returns the provisioning URI for the OTP for `username`. This can then be encoded in a QR code and used
to provision an OTP app like Google Authenticator.
WARNING: response for this endpoint includes the 2FA secret for the user
"""
user = await self.translate_username(username)
user_twofactor_config = await self.middleware.call(
Expand All @@ -36,10 +38,12 @@ async def provisioning_uri(self, username):

return await self.provisioning_uri_internal(username, user_twofactor_config)

@api_method(UserTwofactorConfigArgs, UserTwofactorConfigResult, roles=['ACCOUNT_READ'])
@api_method(UserTwofactorConfigArgs, UserTwofactorConfigResult, private=True)
async def twofactor_config(self, username):
"""
Returns two-factor authentication configuration settings for specified `username`.
WARNING: response for this endpoint includes the 2FA secret for the user
"""
user = await self.translate_username(username)
user_twofactor_config = await self.middleware.call(
Expand All @@ -57,7 +61,7 @@ async def twofactor_config(self, username):
'otp_digits': user_twofactor_config['otp_digits'],
}

@api_method(UserVerifyTwofactorTokenArgs, UserVerifyTwofactorTokenResult, roles=['FULL_ADMIN'])
@api_method(UserVerifyTwofactorTokenArgs, UserVerifyTwofactorTokenResult, private=True)
def verify_twofactor_token(self, username, token):
"""
Returns boolean true if provided `token` is successfully authenticated for `username`.
Expand Down Expand Up @@ -179,4 +183,6 @@ async def renew_2fa_secret(self, app, username, twofactor_options):
# This needs to be reloaded so that user's new secret can be reflected in sshd configuration
await self.middleware.call('service.reload', 'ssh')

return await self.translate_username(username)
user_entry = await self.translate_username(username)
twofactor_config = await self.twofactor_config(username)
return user_entry | {'twofactor_config': twofactor_config}
1 change: 0 additions & 1 deletion tests/unit/test_role_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
READONLY_ADMIN_METHODS = ALL_METHODS - WRITE_METHODS
FULL_ADMIN_STIG = ALL_METHODS - FAKE_METHODS_NOSTIG
EXPECTED_FA_RESOURCES = frozenset({
'user.verify_twofactor_token',
'failover.reboot.other_node',
'truenas.accept_eula',
'filesystem.put',
Expand Down

0 comments on commit ac1f80b

Please sign in to comment.