Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
1 task done
almereyda opened this issue Jun 20, 2024 · 0 comments
Open
1 task done
Assignees
Labels
🐛bug Something isn't working

Comments

@almereyda
Copy link
Contributor

almereyda commented Jun 20, 2024

Is there an existing issue for this?

  • I have searched the existing issues

Current behavior

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:
  1. Inspect the links in the previous section.
  2. 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 almereyda added the 🐛bug Something isn't working label Jun 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants