[IA-3882] Fix some endpoints return 200 with empty value when authentication is required but not satisfied

[bmonjoie]

When logged out or connected with a user account linked to another account, accessing forms, formversions, orgunits, and orgunitypes endpoints return a 200 answer with an empty body when the project requires authentication.

This is an issue when switching projects in the Iaso Demo application and other applications where changing appId is possible (E.g.: Pathways).

By returning a 401 in those cases, the mobile application can adequately handle this and direct the user to the login page.

Related JIRA tickets : IA-3882

Self proofreading checklist

• Did I use eslint and black formatters
• Is my code clear enough and well documented
• Are there enough tests

Changes

Adding a new permission class and applying it where it makes sense.

How to test

Before the fix, accessing the specified endpoint for a project that requires authentication while being logged out would return a 200.
After the fix, it should return a 401.
Same goes when logged in with a user account from another account than the project's account.

Follow the Conventional Commits specification

The merge message of a pull request must follow the Conventional Commits specification.

This convention helps to automatically generate release notes.

Use lowercase for consistency.

Example:

fix: empty instance pop up

Refs: IA-3665

Note that the Jira reference is preceded by a line break.

Both the line break and the Jira reference are entered in the Add an optional extended description… field.

[kemar]

LGTM

[kemar]

This basically does exactly the same thing with slightly less code.

This is just a suggestion, there is absolutely no obligation to merge it.

Suggested change

	class IsAuthenticatedOrReadOnlyWhenNoAuthenticationRequired(permissions.IsAuthenticatedOrReadOnly):
	def has_permission(self, request, view):
	app_id = AppIdSerializer(data=request.query_params).get_app_id(raise_exception=False)
	if app_id is not None:
	try:
	project = Project.objects.get(app_id=app_id)
	if not bool(request.user and request.user.is_authenticated):
	if project.needs_authentication:
	raise NotAuthenticated()
	elif request.user.iaso_profile.account.id != project.account.id:
	raise NotAuthenticated()
	except Project.DoesNotExist:
	return super().has_permission(request, view)
	return super().has_permission(request, view)
	class IsAuthenticatedOrReadOnlyWhenNoAuthenticationRequired(permissions.IsAuthenticatedOrReadOnly):
	def has_permission(self, request, view):
	app_id = AppIdSerializer(data=request.query_params).get_app_id(raise_exception=False)
	if app_id:
	try:
	project = Project.objects.get(app_id=app_id)
	except Project.DoesNotExist:
	return super().has_permission(request, view)
	if not request.user.is_authenticated and project.needs_authentication:
	raise NotAuthenticated()
	if request.user.iaso_profile.account.id != project.account.id:
	raise NotAuthenticated()
	return super().has_permission(request, view)

[tdethier]

I agree

[bmonjoie]

@kemar I believe I wrote it the way I did because, unless I'm mistaken, you will have a crash if the project doesn't require authentication, but the user is anonymous.
Indeed, in case the user is anonymous, and the project doesn't require authentication, the line:

if request.user.iaso_profile.account.id != project.account.id:

will crash with user doesn't have an attribute 'iaso_profile', or am I missing something?

[kemar]

@bmonjoie yup you're right, I went too fast but the idea was to remove a level of nesting to make it easier to read.

[bmonjoie]

I have pushed a rewrite of this class in an attempt to make it more readable.

[kemar]

Thanx @bmonjoie indeed it's more readable! 🙇

[tdethier]

Same as Marco!