[bug]: gitlab oauth incomplete, def authentication_error_code() missing

[almereyda]

Is there an existing issue for this?

• I have searched the existing issues

Current behavior

plane/apiserver/plane/authentication/adapter/oauth.py

Lines 65 to 87 in 94e6fd4

	code = self._provider_error_code()
	raise AuthenticationException(
	error_code=AUTHENTICATION_ERROR_CODES[code],
	error_message=str(code),
	)
	def get_user_response(self):
	try:
	headers = {
	"Authorization": f"Bearer {self.token_data.get('access_token')}"
	}
	response = requests.get(self.get_user_info_url(), headers=headers)
	response.raise_for_status()
	return response.json()
	except requests.RequestException:
	if self.provider == "google":
	code = "GOOGLE_OAUTH_PROVIDER_ERROR"
	elif self.provider == "github":
	code = "GITHUB_OAUTH_PROVIDER_ERROR"
	elif self.provider == "gitlab":
	code = "GITLAB_OAUTH_PROVIDER_ERROR"
	else:
	code = "OAUTH_NOT_CONFIGURED"

calls a method _provider_error_code(), which is not defined in the file, nor anywhere else in the repository.

https://github.com/search?q=repo%3Amakeplane%2Fplane%20_provider_error_code()&type=code

It's part of an effort to bring OAuth-based GitLab authentication to plane.

While some of the early implementations have already been improved, as with 84236f5, others have fallen behind.

_provider_error_code() has been part of the earliest implementation in #4692.

It was reintroduced in #4828

73ea645#diff-e8db188a9821b1c82b6a9e8552288f209242e8c62133bd8b924e42db59411e63R42

73ea645#diff-e8db188a9821b1c82b6a9e8552288f209242e8c62133bd8b924e42db59411e63R42-R50

and renamed to a more canonical version of its name, that caters to it's use

22f09a6

22f09a6#diff-e8db188a9821b1c82b6a9e8552288f209242e8c62133bd8b924e42db59411e63R92

I would suggest to test the implementation and would expect it to fail whenever get_user_token() raises an exception. Is it possible to test for such a case, e.g. in an integration test example?

Completion of this code-path seems suitable to people who find themselves in situations debugging a non-working OAuth authentication configuration of a plane instance.

Steps to reproduce

1. Make yourself known to the course of action and conversation in:

• feat: add GitLab OAuth client #4692
• feat: add GitLab OAuth client #4828 (conflicts don't show anymore, but the diff to that branch gives insights what else to cherry-pick)

2. Inspect the links in the previous section.
3. Follow up with the rationale to change the name of _provider_error_code() to authentication_error_code().

Environment

Deploy preview

Browser

Other

Variant

Self-hosted

Version

v0.22.0-dev

[almereyda]

The method is now available on preview.

plane/apiserver/plane/authentication/adapter/oauth.py

Lines 42 to 50 in 734e920

	def authentication_error_code(self):
	if self.provider == "google":
	return "GOOGLE_OAUTH_PROVIDER_ERROR"
	elif self.provider == "github":
	return "GITHUB_OAUTH_PROVIDER_ERROR"
	elif self.provider == "gitlab":
	return "GITLAB_OAUTH_PROVIDER_ERROR"
	else:
	return "OAUTH_NOT_CONFIGURED"

What is missing to being able to close here from your point of view, @pushya22? Have we observed unintended side-effects from the change?

[pablohashescobar]

@almereyda, the issue has been fixed. Closing the issue.