← 返回首页
chore: add authentication type to GitlabAuthenticationError by JohnVillalovos · Pull Request #1793 · python-gitlab/python-gitlab · GitHub
Skip to content

Navigation Menu

Toggle navigation
Sign in
Appearance settings
Search or jump to...

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Include my email address so I can be contacted
Cancel Submit feedback

Saved searches

Use saved searches to filter your results more quickly

Cancel Create saved search
Appearance settings
Resetting focus

chore: add authentication type to GitlabAuthenticationError #1793

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.

Sign up for GitHub

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

Draft
JohnVillalovos wants to merge 1 commit into
base: main
Choose a base branch
Could not load branches
Branch not found: {{ refName }}
{{ refName }}

Are you sure you want to change the base?

Some commits from the old base branch may be removed from the timeline, and old review comments may become outdated.
Loading Change base
from
+48 −1
Conversation 8 Commits 1 Checks 2 Files changed 3
Draft

chore: add authentication type to GitlabAuthenticationError #1793

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension .py  (3) All 1 file type selected Viewed files
Conversations
Failed to load comments. Retry
Loading
Jump to
Jump to file
Failed to load files. Retry
Loading
Diff view
Unified
Split
Hide whitespace
Apply and reload
Show whitespace
Diff view
Unified
Split
Hide whitespace
Apply and reload
6 changes: 6 additions & 0 deletions gitlab/client.py
Show comments
View file Edit file Delete file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ def __init__(
self.http_password = http_password
self.oauth_token = oauth_token
self.job_token = job_token
self.auth_type = ""
self._set_auth_info()

#: Create a session object for requests
Expand Down Expand Up @@ -487,21 +488,25 @@ def _set_auth_info(self) -> None:
self.headers.pop("Authorization", None)
self.headers["PRIVATE-TOKEN"] = self.private_token
self.headers.pop("JOB-TOKEN", None)
self.auth_type = "private_token"

if self.oauth_token:
self.headers["Authorization"] = f"Bearer {self.oauth_token}"
self.headers.pop("PRIVATE-TOKEN", None)
self.headers.pop("JOB-TOKEN", None)
self.auth_type = "oauth_token"

if self.job_token:
self.headers.pop("Authorization", None)
self.headers.pop("PRIVATE-TOKEN", None)
self.headers["JOB-TOKEN"] = self.job_token
self.auth_type = "job_token"

if self.http_username:
self._http_auth = requests.auth.HTTPBasicAuth(
self.http_username, self.http_password
)
self.auth_type = "password"

def enable_debug(self) -> None:
import logging
Expand Down Expand Up @@ -722,6 +727,7 @@ def http_request(
response_code=result.status_code,
error_message=error_message,
response_body=result.content,
auth_type=self.auth_type,
Copy link
Copy Markdown
Member

nejch Jan 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

I think we could actually reuse self.headers and pass it (or just the dict keys) to the exception to infer the auth type, without adding custom variables here.

http_username/http_password is basically dead code as HTTP Basic auth has been out of GitLab since version 10 so we don't need to worry about that (I actually have a local draft that performs password-based OAuth login from that). So we're left with Private-Token, Job-Token, and Authorization (OAuth bearer) from headers. We could probably just pass the keys from the headers dict to not leak stuff accidentally.
)

raise gitlab.exceptions.GitlabHttpError(
Expand Down
19 changes: 18 additions & 1 deletion gitlab/exceptions.py
Show comments
View file Edit file Delete file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,24 @@ def __str__(self) -> str:


class GitlabAuthenticationError(GitlabError):
pass
def __init__(
self,
error_message: Union[str, bytes] = "",
response_code: Optional[int] = None,
response_body: Optional[bytes] = None,
auth_type: str = "",
) -> None:
super().__init__(
error_message=error_message,
response_code=response_code,
response_body=response_body,
)
self.auth_type = auth_type
Comment on lines +55 to +67
Copy link
Copy Markdown
Member

nejch Jan 9, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

Maybe we could add **kwargs to GitlabError and we don't need to reimplement it here? Just pop it from kwargs?

def __str__(self) -> str:
if self.auth_type:
return f"{super().__str__()}: authentication_type: {self.auth_type}"
Copy link
Copy Markdown
Member

nejch Jan 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

If we decide to infer the auth type automatically I'd maybe reformulate this a bit. Not sure exactly how, just authentication_type sounds like it's a variable name defined/passed somewhere. We can maybe check around how more verbose exceptions do it in cpython or some other libraries.
return super().__str__()


class RedirectError(GitlabError):
Expand Down
24 changes: 24 additions & 0 deletions tests/unit/test_exceptions.py
Show comments
View file Edit file Delete file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,27 @@ def raise_error_from_http_error():
with pytest.raises(TestError) as context:
raise_error_from_http_error()
assert isinstance(context.value.__cause__, exceptions.GitlabHttpError)


def test_gitlabauthenticationerror_with_auth_type():
with pytest.raises(exceptions.GitlabAuthenticationError) as context:
raise exceptions.GitlabAuthenticationError(
error_message="401 Unauthorized",
response_code=401,
response_body=b"bad user",
auth_type="job_token",
)
assert "authentication_type" in str(context.value)
assert "job_token" in str(context.value)
assert "401 Unauthorized" in str(context.value)


def test_gitlabauthenticationerror_no_auth_type():
with pytest.raises(exceptions.GitlabAuthenticationError) as context:
raise exceptions.GitlabAuthenticationError(
error_message="401 Unauthorized",
response_code=401,
response_body=b"bad user",
)
assert "authentication_type" not in str(context.value)
assert "401 Unauthorized" in str(context.value)
Toggle all file notes Toggle all file annotations

Footer

© 2026 GitHub, Inc.