Summary

Consolidate our web login and credentials prompt auth logic.

Review Request #14575 — Created Sept. 8, 2025 and submitted Dec. 9, 2025, 9:54 p.m.

Information

Owner

maubin

Repository

RBTools

Branch

release-5.x

Bugs

Depends On

~~14573~~

Blocks

14576

Reviewers

Groups

rbtools

People

Description

The get_authenticated_session() and BaseCommand.credentials_prompt()

functions both contain the same code for prompting users for their

username and password in the terminal. These need to be merged so we

don't have to maintain two copies of the same code. The functions aren't

direct replacements for one another though, they have different usages.

So instead we pull out the credentials prompt logic into a

credentials_prompt() function, which the above functions can call.
Previously, only get_authenticated_session() contained the logic for

kicking off web-based login. But we want to enable web-based login for

all rbt commands, not just the ones that call get_authenticated_session().

We now pull out the logic into an attempt_web_login() function that

anything can call when they want to authenticate a client using

web-based login.
In the next change we'll make use of this function in a new auth handler

that lets us try web-based login if enabled before falling back on Basic

auth.

Testing Done


Ran unit tests.
Tested using all versions of the web login, open browser, and

  debug/logging options and their .reviewboardrc config values

  on rbt login, rbt post and rbt status.
Tested passing a username, a username and password, and an API

  token to commands.
Tested passing the same auth credentials above to commands while

  also passing --web-login (or setting in in .reviewboardrc), saw

  that the auth credentials were used instead.
Tested rbt post --diff-filename=- with piping a diff into STDIN

  while also passing --web-login. Was able to login using web

  auth before the rest of the command ran.

Commits

Summary	ID
Consolidate our web login and credentials prompt auth logic. The `get_authenticated_session()` and `BaseCommand.credentials_prompt()` functions both contain the same code for prompting users for their username and password in the terminal. These need to be merged so we don't have to maintain two copies of the same code. The functions aren't direct replacements for one another though, they have different usages. So instead we pull out the credentials prompt logic into a `credentials_prompt()` function, which the above functions can call. Previously, only `get_authenticated_session()` contained the logic for kicking off web-based login. But we want to enable web-based login for all rbt commands, not just the ones that call `get_authenticated_session()`. We now pull out the logic into an `attempt_web_login()` function that anything can call when they want to authenticate a client using web-based login. In the next change we'll make use of this function in a new auth handler that lets us try web-based login if enabled before falling back on Basic auth.	19fc9926fa084c087b41aba9109455aa08ac768f

Summary

Consolidate our web login and credentials prompt auth logic.

The `get_authenticated_session()` and `BaseCommand.credentials_prompt()` functions both contain the same code for prompting users for their username and password in the terminal. These need to be merged so we don't have to maintain two copies of the same code. The functions aren't direct replacements for one another though, they have different usages. So instead we pull out the credentials prompt logic into a `credentials_prompt()` function, which the above functions can call. Previously, only `get_authenticated_session()` contained the logic for kicking off web-based login. But we want to enable web-based login for all rbt commands, not just the ones that call `get_authenticated_session()`. We now pull out the logic into an `attempt_web_login()` function that anything can call when they want to authenticate a client using web-based login. In the next change we'll make use of this function in a new auth handler that lets us try web-based login if enabled before falling back on Basic auth.

19fc9926fa084c087b41aba9109455aa08ac768f

Issues

Description	From	Last Updated
There's still a leading blank line here.	chipx86	Sept. 14, 2025, 10:24 p.m.
This is missing docs for server_url and is_retry, and Returns.	chipx86	Sept. 14, 2025, 10:27 p.m.
Let's make all these keyword-only, so we're more free to change these as needs change.	chipx86	Sept. 14, 2025, 10:30 p.m.
This can be (Capabilities \| None) = None.	chipx86	Sept. 14, 2025, 10:35 p.m.
We should check if we're expecting web auth, and allow this to proceed if so.	david	Sept. 18, 2025, 2:01 a.m.
Can you add type hints to this while you're at it?	david	Sept. 18, 2025, 2:05 a.m.
These can use (str \| None) = None	david	Sept. 18, 2025, 2:06 a.m.
Please add a blank line before the return	david	Sept. 18, 2025, 2:07 a.m.
Let's go with 5.4.	chipx86	Oct. 15, 2025, 2:49 a.m.
We should pass server_url as a parameter, rather than using f-strings.	chipx86	Oct. 15, 2025, 2:49 a.m.

flake8 passed.

JSHint passed.

rbtools/commands/base/commands.py (Diff revision 1)
The issue has been resolved. Show all issues
```
There's still a leading blank line here.
```

rbtools/utils/users.py (Diff revision 1)

The issue has been resolved. Show all issues

This is missing docs for server_url and is_retry, and Returns.

rbtools/utils/web_login.py (Diff revision 1)

The issue has been resolved. Show all issues

Let's make all these keyword-only, so we're more free to change these as needs change.

rbtools/utils/web_login.py (Diff revision 1)
The issue has been resolved. Show all issues
```
This can be (Capabilities | None) = None.
```

Change Summary:

Follow modern typing syntax, add missing docs, remove an unecessary newline.

Commits:

	Summary	ID
	Consolidate our web login and credentials prompt auth logic. The `get_authenticated_session()` and `BaseCommand.credentials_prompt()` functions both contain the same code for prompting users for their username and password in the terminal. These need to be merged so we don't have to maintain two copies of the same code. The functions aren't direct replacements for one another though, they have different usages. So instead we pull out the credentials prompt logic into a `credentials_prompt()` function, which the above functions can call. Previously, only `get_authenticated_session()` contained the logic for kicking off web-based login. But we want to enable web-based login for all rbt commands, not just the ones that call `get_authenticated_session()`. We now pull out the logic into an `attempt_web_login()` function that anything can call when they want to authenticate a client using web-based login. In the next change we'll make use of this function in a new auth handler that lets us try web-based login if enabled before falling back on Basic auth.	8017ed8180ca13a888c0d0853f817dfb89f69d43
	Consolidate our web login and credentials prompt auth logic. The `get_authenticated_session()` and `BaseCommand.credentials_prompt()` functions both contain the same code for prompting users for their username and password in the terminal. These need to be merged so we don't have to maintain two copies of the same code. The functions aren't direct replacements for one another though, they have different usages. So instead we pull out the credentials prompt logic into a `credentials_prompt()` function, which the above functions can call. Previously, only `get_authenticated_session()` contained the logic for kicking off web-based login. But we want to enable web-based login for all rbt commands, not just the ones that call `get_authenticated_session()`. We now pull out the logic into an `attempt_web_login()` function that anything can call when they want to authenticate a client using web-based login. In the next change we'll make use of this function in a new auth handler that lets us try web-based login if enabled before falling back on Basic auth.	e94af4b758153d42d6e3345f54746ab287d4a8b0

Diff:

Revision 2 (+374 -224)

Show changes

	rbtools/commands/base/commands.py
	rbtools/utils/users.py
	rbtools/utils/web_login.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

So glad to see this finally happen, this has always bugged me.

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

The issue has been dropped. Show all issues

We should check if we're expecting web auth, and allow this to proceed if so.

maubin Sept. 18, 2025, 2:20 a.m.

If we're expecting web auth, the web auth handler will be called before this credentials prompt, so we would never reach this scenario.

rbtools/utils/users.py (Diff revision 2)

The issue has been dropped. Show all issues

Can you add type hints to this while you're at it?

maubin Sept. 18, 2025, 2:20 a.m.

You already added type hints to this whole file on master :p which is why I left them out here. get_user in particular has a UserItemResource return type which doesn't exist on this branch yet.

rbtools/utils/users.py (Diff revision 2)
The issue has been resolved. Show all issues
```
These can use (str | None) = None
```
rbtools/utils/web_login.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Please add a blank line before the return
```

Testing Done:

		Ran unit tests.
		Tested using all versions of the web login, open browser, and debug/logging options and their `.reviewboardrc` config values on `rbt login`, `rbt post` and `rbt status`.
		Tested passing a username, a username and password, and an API token to commands.
		Tested passing the same auth credentials above to commands while also passing --web-login (or setting in in `.reviewboardrc`), saw that the auth credentials were used instead.
	+	Tested `rbt post --diff-filename=-` with piping a diff into STDIN while also passing `--web-login`. Was able to login using web auth before the rest of the command ran.

Commits:

	Summary	ID
	Consolidate our web login and credentials prompt auth logic. The `get_authenticated_session()` and `BaseCommand.credentials_prompt()` functions both contain the same code for prompting users for their username and password in the terminal. These need to be merged so we don't have to maintain two copies of the same code. The functions aren't direct replacements for one another though, they have different usages. So instead we pull out the credentials prompt logic into a `credentials_prompt()` function, which the above functions can call. Previously, only `get_authenticated_session()` contained the logic for kicking off web-based login. But we want to enable web-based login for all rbt commands, not just the ones that call `get_authenticated_session()`. We now pull out the logic into an `attempt_web_login()` function that anything can call when they want to authenticate a client using web-based login. In the next change we'll make use of this function in a new auth handler that lets us try web-based login if enabled before falling back on Basic auth.	e94af4b758153d42d6e3345f54746ab287d4a8b0
	Consolidate our web login and credentials prompt auth logic. The `get_authenticated_session()` and `BaseCommand.credentials_prompt()` functions both contain the same code for prompting users for their username and password in the terminal. These need to be merged so we don't have to maintain two copies of the same code. The functions aren't direct replacements for one another though, they have different usages. So instead we pull out the credentials prompt logic into a `credentials_prompt()` function, which the above functions can call. Previously, only `get_authenticated_session()` contained the logic for kicking off web-based login. But we want to enable web-based login for all rbt commands, not just the ones that call `get_authenticated_session()`. We now pull out the logic into an `attempt_web_login()` function that anything can call when they want to authenticate a client using web-based login. In the next change we'll make use of this function in a new auth handler that lets us try web-based login if enabled before falling back on Basic auth.	e4266bee5bd93010b864fee3467ddbced6e49192

Diff:

Revision 3 (+376 -224)

Show changes

	rbtools/commands/base/commands.py
	rbtools/utils/users.py
	rbtools/utils/web_login.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Change Summary:

Updated for the new dataclass version of RBClientWebLoginOptions.

Commits:

	Summary	ID
	Consolidate our web login and credentials prompt auth logic. The `get_authenticated_session()` and `BaseCommand.credentials_prompt()` functions both contain the same code for prompting users for their username and password in the terminal. These need to be merged so we don't have to maintain two copies of the same code. The functions aren't direct replacements for one another though, they have different usages. So instead we pull out the credentials prompt logic into a `credentials_prompt()` function, which the above functions can call. Previously, only `get_authenticated_session()` contained the logic for kicking off web-based login. But we want to enable web-based login for all rbt commands, not just the ones that call `get_authenticated_session()`. We now pull out the logic into an `attempt_web_login()` function that anything can call when they want to authenticate a client using web-based login. In the next change we'll make use of this function in a new auth handler that lets us try web-based login if enabled before falling back on Basic auth.	e4266bee5bd93010b864fee3467ddbced6e49192
	Consolidate our web login and credentials prompt auth logic. The `get_authenticated_session()` and `BaseCommand.credentials_prompt()` functions both contain the same code for prompting users for their username and password in the terminal. These need to be merged so we don't have to maintain two copies of the same code. The functions aren't direct replacements for one another though, they have different usages. So instead we pull out the credentials prompt logic into a `credentials_prompt()` function, which the above functions can call. Previously, only `get_authenticated_session()` contained the logic for kicking off web-based login. But we want to enable web-based login for all rbt commands, not just the ones that call `get_authenticated_session()`. We now pull out the logic into an `attempt_web_login()` function that anything can call when they want to authenticate a client using web-based login. In the next change we'll make use of this function in a new auth handler that lets us try web-based login if enabled before falling back on Basic auth.	1d4cb3ab8edabd74c4078416ea050b493b3e56b0

Diff:

Revision 4 (+388 -234)

Show changes

	rbtools/commands/base/commands.py
	rbtools/utils/users.py
	rbtools/utils/web_login.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

rbtools/utils/users.py (Diff revision 4)
The issue has been resolved. Show all issues
```
Let's go with 5.4.
```

rbtools/utils/users.py (Diff revision 4)

The issue has been resolved. Show all issues

We should pass server_url as a parameter, rather than using f-strings.

Commits:

	Summary	ID
	Consolidate our web login and credentials prompt auth logic. The `get_authenticated_session()` and `BaseCommand.credentials_prompt()` functions both contain the same code for prompting users for their username and password in the terminal. These need to be merged so we don't have to maintain two copies of the same code. The functions aren't direct replacements for one another though, they have different usages. So instead we pull out the credentials prompt logic into a `credentials_prompt()` function, which the above functions can call. Previously, only `get_authenticated_session()` contained the logic for kicking off web-based login. But we want to enable web-based login for all rbt commands, not just the ones that call `get_authenticated_session()`. We now pull out the logic into an `attempt_web_login()` function that anything can call when they want to authenticate a client using web-based login. In the next change we'll make use of this function in a new auth handler that lets us try web-based login if enabled before falling back on Basic auth.	1d4cb3ab8edabd74c4078416ea050b493b3e56b0
	Consolidate our web login and credentials prompt auth logic. The `get_authenticated_session()` and `BaseCommand.credentials_prompt()` functions both contain the same code for prompting users for their username and password in the terminal. These need to be merged so we don't have to maintain two copies of the same code. The functions aren't direct replacements for one another though, they have different usages. So instead we pull out the credentials prompt logic into a `credentials_prompt()` function, which the above functions can call. Previously, only `get_authenticated_session()` contained the logic for kicking off web-based login. But we want to enable web-based login for all rbt commands, not just the ones that call `get_authenticated_session()`. We now pull out the logic into an `attempt_web_login()` function that anything can call when they want to authenticate a client using web-based login. In the next change we'll make use of this function in a new auth handler that lets us try web-based login if enabled before falling back on Basic auth.	19fc9926fa084c087b41aba9109455aa08ac768f

Diff:

Revision 5 (+388 -234)

Show changes

	rbtools/commands/base/commands.py
	rbtools/utils/users.py
	rbtools/utils/web_login.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-5.x (1fb0487)