Skip to content

feat: enhance BitbucketServerProvider authentication with username and password fallback #1980

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

boston008
Copy link

@boston008 boston008 commented Aug 7, 2025

User description

feat: enhance BitbucketServerProvider authentication with username and password fallback


PR Type

Enhancement


Description

  • Add username/password authentication fallback for BitbucketServerProvider

  • Maintain bearer token as primary authentication method

  • Improve authentication flexibility for different deployment scenarios


Diagram Walkthrough

flowchart LR
  A["BitbucketServerProvider"] --> B["Check Bearer Token"]
  B --> C["Bearer Token Available?"]
  C -->|Yes| D["Use Token Authentication"]
  C -->|No| E["Use Username/Password Authentication"]
  D --> F["Initialize Bitbucket Client"]
  E --> F
Loading

File Walkthrough

Relevant files
Enhancement
bitbucket_server_provider.py
Enhanced authentication with username/password fallback   

pr_agent/git_providers/bitbucket_server_provider.py

  • Add username and password configuration retrieval from settings
  • Implement conditional authentication logic with bearer token priority
  • Restructure Bitbucket client initialization with fallback mechanism
+15/-3   

Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
✅ No TODO sections
🔒 No security concerns identified
⚡ Recommended focus areas for review

Missing Validation

No validation for username/password authentication scenario. If bearer token is None but username or password are also None, the Bitbucket client initialization will fail silently or with unclear error messages.

else:
    self.bitbucket_client = bitbucket_client or Bitbucket(
        url=self.bitbucket_server_url,
        username=self.username,
        password=self.password
    )
Code Duplication

Bitbucket client initialization is duplicated in both authentication branches. Consider extracting common initialization logic to reduce code duplication.

    self.bitbucket_client = bitbucket_client or Bitbucket(
        url=self.bitbucket_server_url,
        token=self.bearer_token
    )
else:
    self.bitbucket_client = bitbucket_client or Bitbucket(
        url=self.bitbucket_server_url,
        username=self.username,
        password=self.password
    )

Copy link
Contributor

qodo-merge-for-open-source bot commented Aug 7, 2025

PR Code Suggestions ✨

Latest suggestions up to e14beac

CategorySuggestion                                                                                                                                    Impact
Possible issue
Validate fallback credentials presence

Guard against missing credentials when falling back to username/password. If
neither a bearer token nor both username and password are provided, raise a
clear error early to avoid confusing authentication failures later.

pr_agent/git_providers/bitbucket_server_provider.py [41-56]

 #get username and password from settings
 self.username = get_settings().get("BITBUCKET_SERVER.USERNAME", None)
 self.password = get_settings().get("BITBUCKET_SERVER.PASSWORD", None)
 self.bitbucket_server_url = self._parse_bitbucket_server(url=pr_url)
 #if bearer token is provided, use it to authenticate, otherwise use username and password
 if self.bearer_token:
     self.bitbucket_client = bitbucket_client or Bitbucket(
         url=self.bitbucket_server_url,
         token=self.bearer_token
     )
 else:
+    if not self.username or not self.password:
+        raise ValueError("Bitbucket authentication requires either 'BITBUCKET_SERVER.BEARER_TOKEN' or both 'BITBUCKET_SERVER.USERNAME' and 'BITBUCKET_SERVER.PASSWORD'.")
     self.bitbucket_client = bitbucket_client or Bitbucket(
         url=self.bitbucket_server_url,
         username=self.username,
         password=self.password
     )
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: Adding an early check for missing self.username/self.password when no self.bearer_token is provided improves robustness and gives a clear error instead of failing later during Bitbucket auth.

Medium
Validate server URL before auth

Add a check that self.bitbucket_server_url is not None before constructing the
client. If URL parsing fails, raising a clear error prevents subsequent
authentication calls from failing unexpectedly.

pr_agent/git_providers/bitbucket_server_provider.py [45-60]

 #if bearer token is provided, use it to authenticate, otherwise use username and password
+if not self.bitbucket_server_url:
+    raise ValueError("Invalid or missing Bitbucket Server URL parsed from PR URL.")
 if self.bearer_token:
     self.bitbucket_client = bitbucket_client or Bitbucket(
         url=self.bitbucket_server_url,
         token=self.bearer_token
     )
 else:
     self.bitbucket_client = bitbucket_client or Bitbucket(
         url=self.bitbucket_server_url,
         username=self.username,
         password=self.password
     )
 try:
     self.bitbucket_api_version = parse_version(self.bitbucket_client.get("rest/api/1.0/application-properties").get('version'))
 except Exception:
     self.bitbucket_api_version = None
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: Verifying self.bitbucket_server_url before constructing the client prevents hard-to-diagnose failures and surfaces configuration issues earlier with a clear message.

Low
Learned
best practice
Add error handling for client init

Wrap Bitbucket client initialization in a try-except block to handle
authentication or connection failures gracefully. Log a clear error message and
re-raise to aid troubleshooting.

pr_agent/git_providers/bitbucket_server_provider.py [46-56]

-if self.bearer_token:
-    self.bitbucket_client = bitbucket_client or Bitbucket(
-        url=self.bitbucket_server_url,
-        token=self.bearer_token
-    )
-else:
-    self.bitbucket_client = bitbucket_client or Bitbucket(
-        url=self.bitbucket_server_url,
-        username=self.username,
-        password=self.password
-    )
+try:
+    if self.bearer_token:
+        self.bitbucket_client = bitbucket_client or Bitbucket(
+            url=self.bitbucket_server_url,
+            token=self.bearer_token
+        )
+    else:
+        self.bitbucket_client = bitbucket_client or Bitbucket(
+            url=self.bitbucket_server_url,
+            username=self.username,
+            password=self.password
+        )
+except Exception as e:
+    get_logger().error(f"Failed to initialize Bitbucket client for {self.bitbucket_server_url}: {e}")
+    raise
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why:
Relevant best practice - Add proper error handling with try-catch blocks for operations that can fail (e.g., API calls) to prevent crashes and provide meaningful error messages.

Low
  • More
  • Author self-review: I have reviewed the PR code suggestions, and addressed the relevant ones.

Previous suggestions

Suggestions up to commit e14beac
CategorySuggestion                                                                                                                                    Impact
Possible issue
Add authentication credential validation

Add validation to ensure that when falling back to username/password
authentication, both credentials are provided. Without this check, the Bitbucket
client may fail silently or throw unclear errors if only one credential is
missing.

pr_agent/git_providers/bitbucket_server_provider.py [41-56]

 #get username and password from settings
 self.username = get_settings().get("BITBUCKET_SERVER.USERNAME", None)
 self.password = get_settings().get("BITBUCKET_SERVER.PASSWORD", None)
 self.bitbucket_server_url = self._parse_bitbucket_server(url=pr_url)
 #if bearer token is provided, use it to authenticate, otherwise use username and password
 if self.bearer_token:
     self.bitbucket_client = bitbucket_client or Bitbucket(
         url=self.bitbucket_server_url,
         token=self.bearer_token
     )
-else:
+elif self.username and self.password:
     self.bitbucket_client = bitbucket_client or Bitbucket(
         url=self.bitbucket_server_url,
         username=self.username,
         password=self.password
     )
+else:
+    raise ValueError("Authentication failed: Either BEARER_TOKEN or both USERNAME and PASSWORD must be provided")
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies that the code doesn't handle cases where no bearer token is provided and username/password are also missing, which would lead to an authentication failure. Raising a ValueError is a robust way to handle this, making the configuration error explicit.

Medium
Learned
best practice
Fix comment capitalization and punctuation

The comment lacks proper capitalization and punctuation. Comments should follow
standard grammar rules for professional code quality.

pr_agent/git_providers/bitbucket_server_provider.py [41-43]

-#get username and password from settings
+# Get username and password from settings
 self.username = get_settings().get("BITBUCKET_SERVER.USERNAME", None)
 self.password = get_settings().get("BITBUCKET_SERVER.PASSWORD", None)
Suggestion importance[1-10]: 6

__

Why:
Relevant best practice - Fix typos and grammatical errors in documentation, comments, and user-facing text to maintain professional quality and clarity

Low
Fix comment capitalization and punctuation

The comment lacks proper capitalization and punctuation. Comments should start
with a capital letter and end with proper punctuation for consistency.

pr_agent/git_providers/bitbucket_server_provider.py [45-46]

-#if bearer token is provided, use it to authenticate, otherwise use username and password
+# If bearer token is provided, use it to authenticate, otherwise use username and password
 if self.bearer_token:
Suggestion importance[1-10]: 5

__

Why:
Relevant best practice - Fix typos and grammatical errors in documentation, comments, and user-facing text to maintain professional quality and clarity

Low

@mrT23
Copy link
Collaborator

mrT23 commented Aug 7, 2025

/improve

Copy link
Contributor

Persistent suggestions updated to latest commit e14beac

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants