Summary

Fix issues with authenticating to private-API servers.

Review Request #14751 — Created Jan. 7, 2026 and updated Jan. 8, 2026, 5:04 p.m.

Information

Owner

maubin

Repository

RBTools

Branch

master

Bugs

Depends On

Reviewers

Groups

rbtools

People

Description

RBTools assumes that the public API resources would always be public. We
weren't properly handling communication with private-API servers. This
has revealed itself in a few issues:

Web-based login caused a regression. When a command that requires the
API first initializes itself, it creates an API client and fetches the
root resource. That fetch triggers our auth handlers to prompt for
authentication, but the web login handler would break because its
callback expects the root resource and API client to already be set.
This regression was never made public, it comes from recent
work that hasn't been released yet.
If a user is logged out and does rbt logout, they'll be prompted to
login again.
If a user is logged out and does rbt login, they'll successfully
login but we'll print the "You're already logged in" message.

This change addresses those issues and makes sure that we properly
authenticate commands when interacting with private-API servers. We add
a has_session_cookie() method to the API client (and all the way down
to the server class) which returns whether we have a local session
cookie for the server. This information helps us deal with issues 2 and
3. We also update the login/logout commands to needs_api=False to
address issues 2 and 3.

The web login handler no longer assumes that a root resource is set. We
need the root to get the server info, in order to see whether web login
is supported on the server or not. In the case where we're working with a
private-API server while logged out, we'll just assume that the server
supports web login. Even if it doesn't, the auth will fail and we'll
move on to the username/password prompt handler.

Testing Done


Ran unit tests.
Tested login/logout on both private-API servers and public ones.
Tested login with api tokens, username/password, and web login.

Commits

Summary	ID
Fix issues with authenticating to private-API servers. Review Board's API has certain resources that require authentication. But some resources like its root resource, the session resource, and the server resource don't require any. However, there's a server setting that lets you disable anonymous read access to the API. When that is set, every request to the API must be authenticated. RBTools assumes that the public API resources would always be public. We weren't properly handling communication with private-API servers. This has revealed itself in a few issues: 1. Web-based login caused a regression. When a command that requires the API first initializes itself, it creates an API client and fetches the root resource. That fetch triggers our auth handlers to prompt for authentication, but the web login handler would break because its callback expects the root resource and API client to already be set. 2. If a user is logged out and does `rbt logout`, they'll be prompted to login again. 3. If a user is logged out and does `rbt login`, they'll successfully login but we'll print the "You're already logged in" message. This change addresses those issues and makes sure that we properly authenticate commands when interacting with private-API servers. We add a `has_session_cookie()` method to the API client (and all the way down to the server class) which returns whether we have a local session cookie for the server. This information helps us deal with issues 2 and 3. We also update the login/logout commands to needs_api=False to address issues 2 and 3. The web login handler no longer assumes that a root resource is set. We need the root to get the server info, in order to see whether web login is supported on the server or not. In the case where we're working with a private-API server while logged out, we'll just assume that the server supports web login. Even if it doesn't, the auth will fail and we'll move on to the username/password prompt handler.	547598e1918613117d0013ed4c2d51af05eb8aef

Summary

Fix issues with authenticating to private-API servers.

Review Board's API has certain resources that require authentication. But some resources like its root resource, the session resource, and the server resource don't require any. However, there's a server setting that lets you disable anonymous read access to the API. When that is set, every request to the API must be authenticated. RBTools assumes that the public API resources would always be public. We weren't properly handling communication with private-API servers. This has revealed itself in a few issues: 1. Web-based login caused a regression. When a command that requires the API first initializes itself, it creates an API client and fetches the root resource. That fetch triggers our auth handlers to prompt for authentication, but the web login handler would break because its callback expects the root resource and API client to already be set. 2. If a user is logged out and does `rbt logout`, they'll be prompted to login again. 3. If a user is logged out and does `rbt login`, they'll successfully login but we'll print the "You're already logged in" message. This change addresses those issues and makes sure that we properly authenticate commands when interacting with private-API servers. We add a `has_session_cookie()` method to the API client (and all the way down to the server class) which returns whether we have a local session cookie for the server. This information helps us deal with issues 2 and 3. We also update the login/logout commands to needs_api=False to address issues 2 and 3. The web login handler no longer assumes that a root resource is set. We need the root to get the server info, in order to see whether web login is supported on the server or not. In the case where we're working with a private-API server while logged out, we'll just assume that the server supports web login. Even if it doesn't, the auth will fail and we'll move on to the username/password prompt handler.

547598e1918613117d0013ed4c2d51af05eb8aef

Issues

Description	From	Last Updated
Why do we need three different ways to fetch this? Can you add a comment explaining?	david	Jan. 7, 2026, 10:27 p.m.
Typo: instantianting -> instantiating	david	Jan. 7, 2026, 10:28 p.m.
Instead of this, can we update the type hint of get_authenticated_session to not be \| None? According to my reading …	david	Jan. 7, 2026, 10:28 p.m.

flake8 passed.

JSHint passed.

Change Summary:: Fixed a formatting issue in the description.
Description:: Review Board's API has certain resources that require authentication.
But some resources like its root resource, the session resource, and
the server resource don't require any. However, there's a server
setting that lets you disable anonymous read access to the API. When
that is set, every request to the API must be authenticated.

RBTools assumes that the public API resources would always be public. We
weren't properly handling communication with private-API servers. This
has revealed itself in a few issues:

~

Web-based login caused a regression. When a command that requires the
API first initializes itself, it creates an API client and fetches the
root resource. That fetch triggers our auth handlers to prompt for
authentication, but the web login handler would break because its callback expects the root resource and API client to already be set.

~

Web-based login caused a regression. When a command that requires the
API first initializes itself, it creates an API client and fetches the
root resource. That fetch triggers our auth handlers to prompt for
authentication, but the web login handler would break because its
callback expects the root resource and API client to already be set.

If a user is logged out and does rbt logout, they'll be prompted to
login again.

If a user is logged out and does rbt login, they'll successfully
login but we'll print the "You're already logged in" message.

This change addresses those issues and makes sure that we properly
authenticate commands when interacting with private-API servers. We add
a has_session_cookie() method to the API client (and all the way down
to the server class) which returns whether we have a local session
cookie for the server. This information helps us deal with issues 2 and
3. We also update the login/logout commands to needs_api=False to
address issues 2 and 3.

The web login handler no longer assumes that a root resource is set. We
need the root to get the server info, in order to see whether web login
is supported on the server or not. In the case where we're working with a
private-API server while logged out, we'll just assume that the server
supports web login. Even if it doesn't, the auth will fail and we'll
move on to the username/password prompt handler.

Change Summary:: Added a note that the web login regression isn't public.
Description:: Review Board's API has certain resources that require authentication.
But some resources like its root resource, the session resource, and
the server resource don't require any. However, there's a server
setting that lets you disable anonymous read access to the API. When
that is set, every request to the API must be authenticated.

RBTools assumes that the public API resources would always be public. We
weren't properly handling communication with private-API servers. This
has revealed itself in a few issues:

~

Web-based login caused a regression. When a command that requires the
API first initializes itself, it creates an API client and fetches the
root resource. That fetch triggers our auth handlers to prompt for
authentication, but the web login handler would break because its
callback expects the root resource and API client to already be set.

~

Web-based login caused a regression. When a command that requires the
API first initializes itself, it creates an API client and fetches the
root resource. That fetch triggers our auth handlers to prompt for
authentication, but the web login handler would break because its
callback expects the root resource and API client to already be set.
This regression was never made public, it comes from recent
work that hasn't been released yet.

If a user is logged out and does rbt logout, they'll be prompted to
login again.

If a user is logged out and does rbt login, they'll successfully
login but we'll print the "You're already logged in" message.

This change addresses those issues and makes sure that we properly
authenticate commands when interacting with private-API servers. We add
a has_session_cookie() method to the API client (and all the way down
to the server class) which returns whether we have a local session
cookie for the server. This information helps us deal with issues 2 and
3. We also update the login/logout commands to needs_api=False to
address issues 2 and 3.

The web login handler no longer assumes that a root resource is set. We
need the root to get the server info, in order to see whether web login
is supported on the server or not. In the case where we're working with a
private-API server while logged out, we'll just assume that the server
supports web login. Even if it doesn't, the auth will fail and we'll
move on to the username/password prompt handler.

rbtools/api/request.py (Diff revision 1)

The issue has been resolved. Show all issues

Why do we need three different ways to fetch this? Can you add a comment explaining?

maubin Jan. 7, 2026, 10:30 p.m.

Oops, we don't need all three actually. Originally I was just doing req.headers.get('Cookie') which failed because urllib stores cookies in unredirected_hdrs so that they don't get leaked in redirects. I also thought req.get_header('Cookie') was failing to get the cookie but I was mistaken, its source code shows that it looks in both headers and unredirected_hdrs so I can just use that.

rbtools/commands/base/commands.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Typo: instantianting -> instantiating
```

rbtools/commands/login.py (Diff revision 1)

The issue has been dropped. Show all issues

Instead of this, can we update the type hint of get_authenticated_session to not be | None? According to my reading of the code, it always returns a value.

maubin Jan. 7, 2026, 8:19 p.m.

It can be None if auth_required=False and the session isn't authenticated. I'll add some type overloads to deal with that. I'll do it in an upcoming change where I fix up various typing things that got messed up, I think while merging in release-5.x.

Commits:

	Summary	ID
	Fix issues with authenticating to private-API servers. Review Board's API has certain resources that require authentication. But some resources like its root resource, the session resource, and the server resource don't require any. However, there's a server setting that lets you disable anonymous read access to the API. When that is set, every request to the API must be authenticated. RBTools assumes that the public API resources would always be public. We weren't properly handling communication with private-API servers. This has revealed itself in a few issues: 1. Web-based login caused a regression. When a command that requires the API first initializes itself, it creates an API client and fetches the root resource. That fetch triggers our auth handlers to prompt for authentication, but the web login handler would break because its callback expects the root resource and API client to already be set. 2. If a user is logged out and does `rbt logout`, they'll be prompted to login again. 3. If a user is logged out and does `rbt login`, they'll successfully login but we'll print the "You're already logged in" message. This change addresses those issues and makes sure that we properly authenticate commands when interacting with private-API servers. We add a `has_session_cookie()` method to the API client (and all the way down to the server class) which returns whether we have a local session cookie for the server. This information helps us deal with issues 2 and 3. We also update the login/logout commands to needs_api=False to address issues 2 and 3. The web login handler no longer assumes that a root resource is set. We need the root to get the server info, in order to see whether web login is supported on the server or not. In the case where we're working with a private-API server while logged out, we'll just assume that the server supports web login. Even if it doesn't, the auth will fail and we'll move on to the username/password prompt handler.	a5258e8021da6a0797d62f7148fce9d898422dce
	Fix issues with authenticating to private-API servers. Review Board's API has certain resources that require authentication. But some resources like its root resource, the session resource, and the server resource don't require any. However, there's a server setting that lets you disable anonymous read access to the API. When that is set, every request to the API must be authenticated. RBTools assumes that the public API resources would always be public. We weren't properly handling communication with private-API servers. This has revealed itself in a few issues: 1. Web-based login caused a regression. When a command that requires the API first initializes itself, it creates an API client and fetches the root resource. That fetch triggers our auth handlers to prompt for authentication, but the web login handler would break because its callback expects the root resource and API client to already be set. 2. If a user is logged out and does `rbt logout`, they'll be prompted to login again. 3. If a user is logged out and does `rbt login`, they'll successfully login but we'll print the "You're already logged in" message. This change addresses those issues and makes sure that we properly authenticate commands when interacting with private-API servers. We add a `has_session_cookie()` method to the API client (and all the way down to the server class) which returns whether we have a local session cookie for the server. This information helps us deal with issues 2 and 3. We also update the login/logout commands to needs_api=False to address issues 2 and 3. The web login handler no longer assumes that a root resource is set. We need the root to get the server info, in order to see whether web login is supported on the server or not. In the case where we're working with a private-API server while logged out, we'll just assume that the server supports web login. Even if it doesn't, the auth will fail and we'll move on to the username/password prompt handler.	547598e1918613117d0013ed4c2d51af05eb8aef

Diff:

Revision 2 (+958 -72)

Show changes

	rbtools/api/client.py
	rbtools/api/request.py
	rbtools/api/transport/__init__.py
	rbtools/api/transport/sync.py
	rbtools/commands/login.py
	rbtools/commands/logout.py
	rbtools/commands/base/commands.py
	rbtools/commands/tests/test_login.py
	4 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

rbtools/commands/login.py (Diff revision 2)

This will go away in a later change.

Ship it!

```
Ship It!
```

		Review Board's API has certain resources that require authentication.
		But some resources like its root resource, the session resource, and
		the server resource don't require any. However, there's a server
		setting that lets you disable anonymous read access to the API. When
		that is set, every request to the API must be authenticated.

		RBTools assumes that the public API resources would always be public. We
		weren't properly handling communication with private-API servers. This
		has revealed itself in a few issues:

~		Web-based login caused a regression. When a command that requires the API first initializes itself, it creates an API client and fetches the root resource. That fetch triggers our auth handlers to prompt for authentication, but the web login handler would break because its callback expects the root resource and API client to already be set.
	~	Web-based login caused a regression. When a command that requires the API first initializes itself, it creates an API client and fetches the root resource. That fetch triggers our auth handlers to prompt for authentication, but the web login handler would break because its callback expects the root resource and API client to already be set.
		If a user is logged out and does `rbt logout`, they'll be prompted to login again.
		If a user is logged out and does `rbt login`, they'll successfully login but we'll print the "You're already logged in" message.

		This change addresses those issues and makes sure that we properly
		authenticate commands when interacting with private-API servers. We add
		a `has_session_cookie()` method to the API client (and all the way down
		to the server class) which returns whether we have a local session
		cookie for the server. This information helps us deal with issues 2 and
		3. We also update the login/logout commands to needs_api=False to
		address issues 2 and 3.

		The web login handler no longer assumes that a root resource is set. We
		need the root to get the server info, in order to see whether web login
		is supported on the server or not. In the case where we're working with a
		private-API server while logged out, we'll just assume that the server
		supports web login. Even if it doesn't, the auth will fail and we'll
		move on to the username/password prompt handler.