Skip to content

Commit

Permalink
Handle authentication failures when accessing /api/info/.
Browse files Browse the repository at this point in the history
When accessing a password-protected /api/info/ path, we wouldn't gracefully
handle an invalid login. Instead, we would fall back on the old, deprecated
API. This would always fail on RB 1.6, making things very confusing. Now we
handle this better and just simply bail if we can't log in the first time.
  • Loading branch information
chipx86 committed Sep 14, 2011
1 parent 26f9b14 commit e08cc15
Showing 1 changed file with 8 additions and 2 deletions.
10 changes: 8 additions & 2 deletions rbtools/postreview.py
Expand Up @@ -516,7 +516,7 @@ def check_api_version(self):
self.deprecated_api = False
self.root_resource = root_resource
debug('Using the new web API')
return
return True
except APIError, e:
if e.http_status not in (401, 404):
# We shouldn't reach this. If there's a permission denied
Expand All @@ -528,9 +528,12 @@ def check_api_version(self):
# done your http basic auth
die("Unable to access the root /api/ URL on the server.")

return False

# This is an older Review Board server with the old API.
self.deprecated_api = True
debug('Using the deprecated Review Board 1.0 web API')
return True

def login(self, force=False):
"""
Expand Down Expand Up @@ -3949,7 +3952,10 @@ def main():
sys.exit(1)

server = ReviewBoardServer(server_url, repository_info, cookie_file)
server.check_api_version()

# Handle the case where /api/ requires authorization (RBCommons).
if not server.check_api_version():
die("Unable to log in with the supplied username and password.")

if repository_info.supports_changesets:
changenum = tool.get_changenum(args)
Expand Down

2 comments on commit e08cc15

@teki
Copy link

@teki teki commented on e08cc15 Oct 4, 2011

Choose a reason for hiding this comment

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

This is not backward compatible. (at least it is not compatible with RB 1.0.9)
The api request fails with 404 which used to switch back to the old api, but now it just dies.

@chipx86
Copy link
Member Author

@chipx86 chipx86 commented on e08cc15 Oct 4, 2011

Choose a reason for hiding this comment

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

Good to know. If you have a patch for this that you can test with your setup, we'll take it.

I should point out that we're about to release 0.4.0, and it will not be compatible at all with 1.0.x.

Please sign in to comment.