Merge pull request #352 from TeskaLabs/feature/account-webui-migration-tweaks

Account web UI migration tweaks

Author: byewokko

CHANGELOG.md

@@ -3,6 +3,7 @@
 ## v24.06
 
 ### Pre-releases
+- `v24.06-alpha10`
 - `v24.06-beta3`
 - `v24.06-alpha9`
 - `v24.06-alpha8`
@@ -20,13 +21,15 @@
 - Disable special characters in tenant ID (#349, `v24.06-alpha6`)
 
 ### Fix
+- Better TOTP error responses (#352, `v24.06-alpha10`)
 - Fix resource editability (#355, `v24.06-alpha9`)
 - Make FIDO MDS request non-blocking using TaskService (#354, `v24.06-alpha8`)
 - Improve error handling in FIDO MDS (#351, `v24.06-alpha5`)
 - Fix typo in last login endpoint path (#346, `v24.06-alpha4`)
 - Fix the initialization of NoTenantsError (#346, `v24.06-alpha2`)
 
 ### Features
+- External login provider label contains just the display name (#352, `v24.06-alpha10`)
 - ElasticSearch index and Kibana space authorization (#353, `v24.06-alpha7.2`)
 - Disable special characters in tenant ID (#349, `v24.06-alpha6`)
 - New paths for account management endpoints (#343, `v24.06-alpha3`)

seacatauth/authn/service.py

@@ -279,6 +279,13 @@ async def _prepare_login_descriptors(
 	def get_login_factor(self, factor_type):
 		return self.LoginFactors[factor_type]
 
+	async def get_eligible_factors(self, credentials_id: str):
+		return [
+			factor.Type
+			for factor in self.LoginFactors.values()
+			if await factor.is_eligible({"credentials_id": credentials_id})
+		]
+
 	def create_login_factor(self, factor_config):
 		self.LoginFactors[factor_config["type"]] = login_factor_builder(self, factor_config)
 		return self.LoginFactors[factor_config["type"]]

seacatauth/authn/webauthn/handler.py

@@ -140,7 +140,8 @@ async def list_credentials(self, request, *, credentials_id):
 		"properties": {
 			"name": {
 				"type": "string",
-				"pattern": "^[a-z][a-z0-9._-]{0,128}[a-z0-9]$"
+				"minLength": 3,
+				"maxLength": 128,
 			},
 		}
 	})

seacatauth/cookie/service.py

@@ -191,11 +191,17 @@ async def create_cookie_client_session(
 			])
 
 		if "profile" in scope or "userinfo:authn" in scope or "userinfo:*" in scope:
+			external_login_service = self.App.get_service("seacatauth.ExternalLoginService")
+			available_factors = await self.AuthenticationService.get_eligible_factors(root_session.Credentials.Id)
+			available_external_logins = {
+				result["t"]: result["s"]
+				for result in await external_login_service.list(root_session.Credentials.Id)
+			}
 			session_builders.append([
 				(SessionAdapter.FN.Authentication.LoginDescriptor, root_session.Authentication.LoginDescriptor),
 				(SessionAdapter.FN.Authentication.LoginFactors, root_session.Authentication.LoginFactors),
-				(SessionAdapter.FN.Authentication.ExternalLoginOptions, root_session.Authentication.ExternalLoginOptions),
-				(SessionAdapter.FN.Authentication.AvailableFactors, root_session.Authentication.AvailableFactors),
+				(SessionAdapter.FN.Authentication.AvailableFactors, available_factors),
+				(SessionAdapter.FN.Authentication.ExternalLoginOptions, available_external_logins),
 			])
 
 		if root_session.TrackId is not None:

seacatauth/credentials/schemas.py

@@ -13,13 +13,15 @@
 		"description": "Email address",
 		"anyOf": [
 			{"type": "null"},
+			{"type": "string", "const": ""},
 			{"type": "string", "format": "email"},
 		],
 	},
 	"phone": {
 		"description": "Mobile number",
 		"anyOf": [
 			{"type": "null"},
+			{"type": "string", "const": ""},
 			{"type": "string", "pattern": r"^\+?[0-9 ]+$"},
 		],
 	},

seacatauth/exceptions.py

@@ -123,13 +123,22 @@ def __init__(self, credentials_id, tenant, *args):
 		super().__init__("Credentials do not have the tenant assigned.", *args)
 
 
-class TOTPNotActiveError(SeacatAuthError):
+class TOTPActivationError(SeacatAuthError):
 	"""
-	Credentials do not have any registered TOTP secret
+	Failed to activate TOTP
 	"""
-	def __init__(self, credential_id: str):
-		self.CredentialID: str = credential_id
-		super().__init__("TOTP not active for credentials.")
+	def __init__(self, message: str, credentials_id: str):
+		self.CredentialsID: str = credentials_id
+		super().__init__(message)
+
+
+class TOTPDeactivationError(SeacatAuthError):
+	"""
+	Failed to deactivate TOTP
+	"""
+	def __init__(self, message: str, credentials_id: str):
+		self.CredentialsID: str = credentials_id
+		super().__init__(message)
 
 
 class ClientResponseError(SeacatAuthError):

seacatauth/external_login/providers/appleid.py

@@ -35,7 +35,7 @@ class AppleIDOAuth2Login(GenericOAuth2Login):
 		"token_endpoint": "https://appleid.apple.com/auth/token",
 		"jwks_uri": "https://appleid.apple.com/auth/keys",
 		"scope": "name email",
-		"label": "Sign in with Apple",
+		"label": "AppleID",
 	}
 
 	def __init__(self, external_login_svc, config_section_name, config=None):

seacatauth/external_login/providers/facebook.py

@@ -33,7 +33,7 @@ class FacebookOAuth2Login(GenericOAuth2Login):
 		"response_type": "code",
 		"scope": "public_profile",
 		"fields": "id,name,email",
-		"label": "Sign in with Facebook",
+		"label": "Facebook",
 	}
 
 	def __init__(self, external_login_svc, config_section_name):

seacatauth/external_login/providers/github.py

@@ -27,7 +27,7 @@ class GitHubOAuth2Login(GenericOAuth2Login):
 		"userinfo_endpoint": "https://api.github.com/user",
 		"user_emails_endpoint": "https://api.github.com/user/emails",
 		"scope": "user:email",  # Scope is not used
-		"label": "Sign in with Github",
+		"label": "Github",
 	}
 
 	def __init__(self, external_login_svc, config_section_name):

seacatauth/external_login/providers/google.py

@@ -18,5 +18,5 @@ class GoogleOAuth2Login(GenericOAuth2Login):
 		"authorization_endpoint": "https://accounts.google.com/o/oauth2/auth",
 		"token_endpoint": "https://accounts.google.com/o/oauth2/token",
 		"scope": "openid profile email",
-		"label": "Sign in with Google",
+		"label": "Google",
 	}

seacatauth/external_login/providers/mojeid.py

@@ -18,7 +18,7 @@ class MojeIDOAuth2Login(GenericOAuth2Login):
 		"authorization_endpoint": "https://mojeid.cz/oidc/authorization/",
 		"token_endpoint": "https://mojeid.cz/oidc/token/",
 		"scope": "openid email phone",
-		"label": "Sign in with MojeID",
+		"label": "MojeID",
 	}
 
 	# TODO: MojeID provides extensive settings for encryption algorithms and other features.

seacatauth/external_login/providers/office365.py

@@ -20,7 +20,7 @@ class Office365OAuth2Login(GenericOAuth2Login):
 		"token_endpoint": "https://login.microsoftonline.com/{tenant_id}/oauth2/v2.0/token",
 		"tenant_id": "",
 		"scope": "openid",
-		"label": "Sign in with Office365",
+		"label": "Office365",
 	}
 
 	def __init__(self, external_login_svc, config_section_name):

seacatauth/feature/handler.py

@@ -23,6 +23,7 @@ def __init__(self, app, feture_svc):
 
 		web_app = app.WebContainer.WebApp
 		web_app.router.add_get("/public/features", self.get_features)
+		web_app.router.add_get("/account/features", self.get_features)
 
 		# Public endpoints
 		web_app_public = app.PublicWebContainer.WebApp

seacatauth/openidconnect/service.py

@@ -256,14 +256,18 @@ async def create_oidc_session(
 		]
 
 		if "profile" in scope or "userinfo:authn" in scope or "userinfo:*" in scope:
+			authentication_service = self.App.get_service("seacatauth.AuthenticationService")
+			external_login_service = self.App.get_service("seacatauth.ExternalLoginService")
+			available_factors = await authentication_service.get_eligible_factors(root_session.Credentials.Id)
+			available_external_logins = {
+				result["t"]: result["s"]
+				for result in await external_login_service.list(root_session.Credentials.Id)
+			}
 			session_builders.append([
 				(SessionAdapter.FN.Authentication.LoginDescriptor, root_session.Authentication.LoginDescriptor),
 				(SessionAdapter.FN.Authentication.LoginFactors, root_session.Authentication.LoginFactors),
-				(SessionAdapter.FN.Authentication.AvailableFactors, root_session.Authentication.AvailableFactors),
-				(
-					SessionAdapter.FN.Authentication.ExternalLoginOptions,
-					root_session.Authentication.ExternalLoginOptions
-				),
+				(SessionAdapter.FN.Authentication.AvailableFactors, available_factors),
+				(SessionAdapter.FN.Authentication.ExternalLoginOptions, available_external_logins),
 			])
 
 		if "batman" in scope:

seacatauth/otp/handler.py

@@ -4,7 +4,7 @@
 import asab.web.rest
 
 from ..decorators import access_control
-from ..exceptions import TOTPNotActiveError
+from .. import exceptions
 
 #
 
@@ -28,18 +28,18 @@ def __init__(self, app, otp_svc):
 
 		web_app = app.WebContainer.WebApp
 		web_app.router.add_get("/account/totp", self.prepare_totp_if_not_active)
-		web_app.router.add_put("/account/set-totp", self.set_totp)
-		web_app.router.add_put("/account/unset-totp", self.unset_totp)
+		web_app.router.add_put("/account/set-totp", self.activate_totp)
+		web_app.router.add_put("/account/unset-totp", self.deactivate_totp)
 
 		# Back-compat; To be removed in next major version
 		# >>>
 		web_app.router.add_get("/public/totp", self.prepare_totp_if_not_active)
-		web_app.router.add_put("/public/set-totp", self.set_totp)
-		web_app.router.add_put("/public/unset-totp", self.unset_totp)
+		web_app.router.add_put("/public/set-totp", self.activate_totp)
+		web_app.router.add_put("/public/unset-totp", self.deactivate_totp)
 
 		web_app_public.router.add_get("/public/totp", self.prepare_totp_if_not_active)
-		web_app_public.router.add_put("/public/set-totp", self.set_totp)
-		web_app_public.router.add_put("/public/unset-totp", self.unset_totp)
+		web_app_public.router.add_put("/public/set-totp", self.activate_totp)
+		web_app_public.router.add_put("/public/unset-totp", self.deactivate_totp)
 		# <<<
 
 	@access_control()
@@ -71,27 +71,30 @@ async def prepare_totp_if_not_active(self, request, *, credentials_id):
 		}
 	})
 	@access_control()
-	async def set_totp(self, request, *, credentials_id, json_data):
+	async def activate_totp(self, request, *, credentials_id, json_data):
 		"""
 		Activate TOTP for the current user
 
 		This requires that a TOTP secret is already prepared for the user.
 		"""
 		otp = json_data.get("otp")
-		response = await self.OTPService.activate_prepared_totp(request.Session, credentials_id, otp)
-		return asab.web.rest.json_response(request, response)
+		try:
+			await self.OTPService.activate_prepared_totp(request.Session, credentials_id, otp)
+		except exceptions.TOTPActivationError:
+			return asab.web.rest.json_response(request, {"result": "FAILED"}, status=400)
+		return asab.web.rest.json_response(request, {"result": "OK"})
 
 
 	@access_control()
-	async def unset_totp(self, request, *, credentials_id):
+	async def deactivate_totp(self, request, *, credentials_id):
 		"""
 		Deactivate TOTP for the current user
 
 		The user's TOTP secret is deleted.
 		"""
 		try:
 			await self.OTPService.deactivate_totp(credentials_id)
-		except TOTPNotActiveError:
+		except exceptions.TOTPDeactivationError:
 			return asab.web.rest.json_response(request, {"result": "FAILED"}, status=400)
 
 		return asab.web.rest.json_response(request, {"result": "OK"})

seacatauth/otp/service.py

@@ -8,7 +8,7 @@
 import asab.storage
 
 from typing import Optional
-from ..exceptions import TOTPNotActiveError
+from .. import exceptions
 from ..events import EventTypes
 
 #
@@ -41,21 +41,23 @@ async def _on_housekeeping(self, event_name):
 		await self._delete_expired_totp_secrets()
 
 
-	async def deactivate_totp(self, credential_id: str):
+	async def deactivate_totp(self, credentials_id: str):
 		"""
 		Delete active TOTP secret for requested credentials.
 		"""
-		if not await self.has_activated_totp(credential_id):
-			raise TOTPNotActiveError(credential_id)
+		if not await self.has_activated_totp(credentials_id):
+			L.log(asab.LOG_NOTICE, "Cannot deactivate TOTP because it is already active.", struct_data={
+				"cid": credentials_id})
+			raise exceptions.TOTPDeactivationError("TOTP is not active.", credentials_id)
 		try:
-			await self.StorageService.delete(collection=self.TOTPCollection, obj_id=credential_id)
+			await self.StorageService.delete(collection=self.TOTPCollection, obj_id=credentials_id)
 		except KeyError:
 			# TOTP are stored in the old self.PreparedTOTPCollection -> only for backward compatibility
 			pass
 
 
-		provider = self.CredentialsService.get_provider(credential_id)
-		await provider.update(credential_id, {
+		provider = self.CredentialsService.get_provider(credentials_id)
+		await provider.update(credentials_id, {
 			"__totp": None
 		})
 
@@ -86,29 +88,32 @@ async def activate_prepared_totp(self, session, credentials_id: str, request_otp
 		Requires entering the generated OTP to succeed.
 		"""
 		if await self.has_activated_totp(credentials_id):
-			return {"result": "FAILED"}
+			L.log(asab.LOG_NOTICE, "Cannot activate TOTP because it is already active.", struct_data={
+				"cid": credentials_id})
+			raise exceptions.TOTPActivationError("TOTP is already active.", credentials_id)
 
 		try:
 			secret = await self._get_prepared_totp_secret_by_session_id(session.SessionId)
 		except KeyError:
-			# TOTP secret has not been initialized or has expired
-			return {"result": "FAILED"}
+			L.log(asab.LOG_NOTICE, "Cannot activate TOTP because the secret is not ready or has expired.", struct_data={
+				"cid": credentials_id})
+			raise exceptions.TOTPActivationError("TOTP secret is not ready, or possibly has expired.", credentials_id)
 
 		totp = pyotp.TOTP(secret)
 		if totp.verify(request_otp) is False:
 			# TOTP secret does not match
-			return {"result": "FAILED"}
+			L.log(asab.LOG_NOTICE, "Cannot activate TOTP because the verification failed.", struct_data={
+				"cid": credentials_id})
+			raise exceptions.TOTPActivationError("TOTP verification failed.", credentials_id)
 
 		# Store secret in its own dedicated collection
 		upsertor = self.StorageService.upsertor(collection=self.TOTPCollection, obj_id=credentials_id)
 		upsertor.set("__totp", secret.encode("ascii"), encrypt=True)
 		await upsertor.execute(event_type=EventTypes.TOTP_REGISTERED)
-		L.log(asab.LOG_NOTICE, "TOTP secret registered.", struct_data={"cid": credentials_id})
+		L.log(asab.LOG_NOTICE, "TOTP activated.", struct_data={"cid": credentials_id})
 
 		await self._delete_prepared_totp_secret(session.SessionId)
 
-		return {"result": "OK"}
-
 
 	async def _create_totp_secret(self, session_id: str) -> str:
 		"""
@@ -126,7 +131,7 @@ async def _create_totp_secret(self, session_id: str) -> str:
 		expires: datetime.datetime = datetime.datetime.now(datetime.timezone.utc) + self.RegistrationTimeout
 		upsertor.set("exp", expires)
 
-		secret: str = pyotp.random_base32().encode("ascii")
+		secret = pyotp.random_base32().encode("ascii")
 		upsertor.set("__s", secret, encrypt=True)
 
 		await upsertor.execute(event_type=EventTypes.TOTP_CREATED)

seacatauth/session/builders.py

@@ -70,10 +70,7 @@ def authentication_session_builder(login_descriptor):
 
 
 async def available_factors_session_builder(authentication_service, credentials_id):
-	factors = []
-	for factor in authentication_service.LoginFactors.values():
-		if await factor.is_eligible({"credentials_id": credentials_id}):
-			factors.append(factor.Type)
+	factors = await authentication_service.get_eligible_factors(credentials_id)
 	return ((SessionAdapter.FN.Authentication.AvailableFactors, factors),)