core: rework recovery API to return better error messages (#8655)

Signed-off-by: Jens Langhammer <jens@goauthentik.io>
This commit is contained in:
Jens L
2024-02-22 22:57:47 +01:00
committed by GitHub
parent a82af054a4
commit f740ba0ffe
6 changed files with 40 additions and 50 deletions

View File

@ -397,15 +397,14 @@ class UserViewSet(UsedByMixin, ModelViewSet):
def get_queryset(self): # pragma: no cover def get_queryset(self): # pragma: no cover
return User.objects.all().exclude_anonymous().prefetch_related("ak_groups") return User.objects.all().exclude_anonymous().prefetch_related("ak_groups")
def _create_recovery_link(self) -> tuple[Optional[str], Optional[Token]]: def _create_recovery_link(self) -> tuple[str, Token]:
"""Create a recovery link (when the current brand has a recovery flow set), """Create a recovery link (when the current brand has a recovery flow set),
that can either be shown to an admin or sent to the user directly""" that can either be shown to an admin or sent to the user directly"""
brand: Brand = self.request._request.brand brand: Brand = self.request._request.brand
# Check that there is a recovery flow, if not return an error # Check that there is a recovery flow, if not return an error
flow = brand.flow_recovery flow = brand.flow_recovery
if not flow: if not flow:
LOGGER.debug("No recovery flow set") raise ValidationError({"non_field_errors": "No recovery flow set."})
return None, None
user: User = self.get_object() user: User = self.get_object()
planner = FlowPlanner(flow) planner = FlowPlanner(flow)
planner.allow_empty_flows = True planner.allow_empty_flows = True
@ -417,8 +416,7 @@ class UserViewSet(UsedByMixin, ModelViewSet):
}, },
) )
except FlowNonApplicableException: except FlowNonApplicableException:
LOGGER.warning("Recovery flow not applicable to user") raise ValidationError({"non_field_errors": "Recovery flow not applicable to user"})
return None, None
token, __ = FlowToken.objects.update_or_create( token, __ = FlowToken.objects.update_or_create(
identifier=f"{user.uid}-password-reset", identifier=f"{user.uid}-password-reset",
defaults={ defaults={
@ -563,16 +561,13 @@ class UserViewSet(UsedByMixin, ModelViewSet):
@extend_schema( @extend_schema(
responses={ responses={
"200": LinkSerializer(many=False), "200": LinkSerializer(many=False),
"404": LinkSerializer(many=False),
}, },
request=None,
) )
@action(detail=True, pagination_class=None, filter_backends=[]) @action(detail=True, pagination_class=None, filter_backends=[], methods=["POST"])
def recovery(self, request: Request, pk: int) -> Response: def recovery(self, request: Request, pk: int) -> Response:
"""Create a temporary link that a user can use to recover their accounts""" """Create a temporary link that a user can use to recover their accounts"""
link, _ = self._create_recovery_link() link, _ = self._create_recovery_link()
if not link:
LOGGER.debug("Couldn't create token")
return Response({"link": ""}, status=404)
return Response({"link": link}) return Response({"link": link})
@permission_required("authentik_core.reset_user_password") @permission_required("authentik_core.reset_user_password")
@ -587,27 +582,24 @@ class UserViewSet(UsedByMixin, ModelViewSet):
], ],
responses={ responses={
"204": OpenApiResponse(description="Successfully sent recover email"), "204": OpenApiResponse(description="Successfully sent recover email"),
"404": OpenApiResponse(description="Bad request"),
}, },
request=None,
) )
@action(detail=True, pagination_class=None, filter_backends=[]) @action(detail=True, pagination_class=None, filter_backends=[], methods=["POST"])
def recovery_email(self, request: Request, pk: int) -> Response: def recovery_email(self, request: Request, pk: int) -> Response:
"""Create a temporary link that a user can use to recover their accounts""" """Create a temporary link that a user can use to recover their accounts"""
for_user: User = self.get_object() for_user: User = self.get_object()
if for_user.email == "": if for_user.email == "":
LOGGER.debug("User doesn't have an email address") LOGGER.debug("User doesn't have an email address")
return Response(status=404) raise ValidationError({"non_field_errors": "User does not have an email address set."})
link, token = self._create_recovery_link() link, token = self._create_recovery_link()
if not link:
LOGGER.debug("Couldn't create token")
return Response(status=404)
# Lookup the email stage to assure the current user can access it # Lookup the email stage to assure the current user can access it
stages = get_objects_for_user( stages = get_objects_for_user(
request.user, "authentik_stages_email.view_emailstage" request.user, "authentik_stages_email.view_emailstage"
).filter(pk=request.query_params.get("email_stage")) ).filter(pk=request.query_params.get("email_stage"))
if not stages.exists(): if not stages.exists():
LOGGER.debug("Email stage does not exist/user has no permissions") LOGGER.debug("Email stage does not exist/user has no permissions")
return Response(status=404) raise ValidationError({"non_field_errors": "Email stage does not exist."})
email_stage: EmailStage = stages.first() email_stage: EmailStage = stages.first()
message = TemplateEmailMessage( message = TemplateEmailMessage(
subject=_(email_stage.subject), subject=_(email_stage.subject),

View File

@ -60,10 +60,11 @@ class TestUsersAPI(APITestCase):
def test_recovery_no_flow(self): def test_recovery_no_flow(self):
"""Test user recovery link (no recovery flow set)""" """Test user recovery link (no recovery flow set)"""
self.client.force_login(self.admin) self.client.force_login(self.admin)
response = self.client.get( response = self.client.post(
reverse("authentik_api:user-recovery", kwargs={"pk": self.user.pk}) reverse("authentik_api:user-recovery", kwargs={"pk": self.user.pk})
) )
self.assertEqual(response.status_code, 404) self.assertEqual(response.status_code, 400)
self.assertJSONEqual(response.content, {"non_field_errors": "No recovery flow set."})
def test_set_password(self): def test_set_password(self):
"""Test Direct password set""" """Test Direct password set"""
@ -84,7 +85,7 @@ class TestUsersAPI(APITestCase):
brand.flow_recovery = flow brand.flow_recovery = flow
brand.save() brand.save()
self.client.force_login(self.admin) self.client.force_login(self.admin)
response = self.client.get( response = self.client.post(
reverse("authentik_api:user-recovery", kwargs={"pk": self.user.pk}) reverse("authentik_api:user-recovery", kwargs={"pk": self.user.pk})
) )
self.assertEqual(response.status_code, 200) self.assertEqual(response.status_code, 200)
@ -92,16 +93,20 @@ class TestUsersAPI(APITestCase):
def test_recovery_email_no_flow(self): def test_recovery_email_no_flow(self):
"""Test user recovery link (no recovery flow set)""" """Test user recovery link (no recovery flow set)"""
self.client.force_login(self.admin) self.client.force_login(self.admin)
response = self.client.get( response = self.client.post(
reverse("authentik_api:user-recovery-email", kwargs={"pk": self.user.pk}) reverse("authentik_api:user-recovery-email", kwargs={"pk": self.user.pk})
) )
self.assertEqual(response.status_code, 404) self.assertEqual(response.status_code, 400)
self.assertJSONEqual(
response.content, {"non_field_errors": "User does not have an email address set."}
)
self.user.email = "foo@bar.baz" self.user.email = "foo@bar.baz"
self.user.save() self.user.save()
response = self.client.get( response = self.client.post(
reverse("authentik_api:user-recovery-email", kwargs={"pk": self.user.pk}) reverse("authentik_api:user-recovery-email", kwargs={"pk": self.user.pk})
) )
self.assertEqual(response.status_code, 404) self.assertEqual(response.status_code, 400)
self.assertJSONEqual(response.content, {"non_field_errors": "No recovery flow set."})
def test_recovery_email_no_stage(self): def test_recovery_email_no_stage(self):
"""Test user recovery link (no email stage)""" """Test user recovery link (no email stage)"""
@ -112,10 +117,11 @@ class TestUsersAPI(APITestCase):
brand.flow_recovery = flow brand.flow_recovery = flow
brand.save() brand.save()
self.client.force_login(self.admin) self.client.force_login(self.admin)
response = self.client.get( response = self.client.post(
reverse("authentik_api:user-recovery-email", kwargs={"pk": self.user.pk}) reverse("authentik_api:user-recovery-email", kwargs={"pk": self.user.pk})
) )
self.assertEqual(response.status_code, 404) self.assertEqual(response.status_code, 400)
self.assertJSONEqual(response.content, {"non_field_errors": "Email stage does not exist."})
def test_recovery_email(self): def test_recovery_email(self):
"""Test user recovery link""" """Test user recovery link"""
@ -129,7 +135,7 @@ class TestUsersAPI(APITestCase):
stage = EmailStage.objects.create(name="email") stage = EmailStage.objects.create(name="email")
self.client.force_login(self.admin) self.client.force_login(self.admin)
response = self.client.get( response = self.client.post(
reverse( reverse(
"authentik_api:user-recovery-email", "authentik_api:user-recovery-email",
kwargs={"pk": self.user.pk}, kwargs={"pk": self.user.pk},

View File

@ -4896,8 +4896,8 @@ paths:
$ref: '#/components/schemas/GenericError' $ref: '#/components/schemas/GenericError'
description: '' description: ''
/core/users/{id}/recovery/: /core/users/{id}/recovery/:
get: post:
operationId: core_users_recovery_retrieve operationId: core_users_recovery_create
description: Create a temporary link that a user can use to recover their accounts description: Create a temporary link that a user can use to recover their accounts
parameters: parameters:
- in: path - in: path
@ -4917,12 +4917,6 @@ paths:
schema: schema:
$ref: '#/components/schemas/Link' $ref: '#/components/schemas/Link'
description: '' description: ''
'404':
content:
application/json:
schema:
$ref: '#/components/schemas/Link'
description: ''
'400': '400':
content: content:
application/json: application/json:
@ -4936,8 +4930,8 @@ paths:
$ref: '#/components/schemas/GenericError' $ref: '#/components/schemas/GenericError'
description: '' description: ''
/core/users/{id}/recovery_email/: /core/users/{id}/recovery_email/:
get: post:
operationId: core_users_recovery_email_retrieve operationId: core_users_recovery_email_create
description: Create a temporary link that a user can use to recover their accounts description: Create a temporary link that a user can use to recover their accounts
parameters: parameters:
- in: query - in: query
@ -4958,8 +4952,6 @@ paths:
responses: responses:
'204': '204':
description: Successfully sent recover email description: Successfully sent recover email
'404':
description: Bad request
'400': '400':
content: content:
application/json: application/json:
@ -17893,7 +17885,7 @@ paths:
schema: schema:
type: string type: string
format: uuid format: uuid
description: A UUID string identifying this connection token. description: A UUID string identifying this RAC Connection token.
required: true required: true
tags: tags:
- rac - rac
@ -17927,7 +17919,7 @@ paths:
schema: schema:
type: string type: string
format: uuid format: uuid
description: A UUID string identifying this connection token. description: A UUID string identifying this RAC Connection token.
required: true required: true
tags: tags:
- rac - rac
@ -17967,7 +17959,7 @@ paths:
schema: schema:
type: string type: string
format: uuid format: uuid
description: A UUID string identifying this connection token. description: A UUID string identifying this RAC Connection token.
required: true required: true
tags: tags:
- rac - rac
@ -18006,7 +17998,7 @@ paths:
schema: schema:
type: string type: string
format: uuid format: uuid
description: A UUID string identifying this connection token. description: A UUID string identifying this RAC Connection token.
required: true required: true
tags: tags:
- rac - rac
@ -18037,7 +18029,7 @@ paths:
schema: schema:
type: string type: string
format: uuid format: uuid
description: A UUID string identifying this connection token. description: A UUID string identifying this RAC Connection token.
required: true required: true
tags: tags:
- rac - rac

View File

@ -305,7 +305,7 @@ export class RelatedUserList extends WithBrandConfig(WithCapabilitiesConfig(Tabl
class="pf-m-secondary" class="pf-m-secondary"
.apiRequest=${() => { .apiRequest=${() => {
return new CoreApi(DEFAULT_CONFIG) return new CoreApi(DEFAULT_CONFIG)
.coreUsersRecoveryRetrieve({ .coreUsersRecoveryCreate({
id: item.pk, id: item.pk,
}) })
.then((rec) => { .then((rec) => {

View File

@ -42,7 +42,7 @@ import { CoreApi, ResponseError, SessionUser, User, UserPath } from "@goauthenti
export const requestRecoveryLink = (user: User) => export const requestRecoveryLink = (user: User) =>
new CoreApi(DEFAULT_CONFIG) new CoreApi(DEFAULT_CONFIG)
.coreUsersRecoveryRetrieve({ .coreUsersRecoveryCreate({
id: user.pk, id: user.pk,
}) })
.then((rec) => .then((rec) =>

View File

@ -10,7 +10,7 @@ import { customElement, property } from "lit/decorators.js";
import { import {
CoreApi, CoreApi,
CoreUsersRecoveryEmailRetrieveRequest, CoreUsersRecoveryEmailCreateRequest,
Stage, Stage,
StagesAllListRequest, StagesAllListRequest,
StagesApi, StagesApi,
@ -18,7 +18,7 @@ import {
} from "@goauthentik/api"; } from "@goauthentik/api";
@customElement("ak-user-reset-email-form") @customElement("ak-user-reset-email-form")
export class UserResetEmailForm extends Form<CoreUsersRecoveryEmailRetrieveRequest> { export class UserResetEmailForm extends Form<CoreUsersRecoveryEmailCreateRequest> {
@property({ attribute: false }) @property({ attribute: false })
user!: User; user!: User;
@ -26,9 +26,9 @@ export class UserResetEmailForm extends Form<CoreUsersRecoveryEmailRetrieveReque
return msg("Successfully sent email."); return msg("Successfully sent email.");
} }
async send(data: CoreUsersRecoveryEmailRetrieveRequest): Promise<void> { async send(data: CoreUsersRecoveryEmailCreateRequest): Promise<void> {
data.id = this.user.pk; data.id = this.user.pk;
return new CoreApi(DEFAULT_CONFIG).coreUsersRecoveryEmailRetrieve(data); return new CoreApi(DEFAULT_CONFIG).coreUsersRecoveryEmailCreate(data);
} }
renderForm(): TemplateResult { renderForm(): TemplateResult {