Summary

Fixed HTTP Digest backend unauthenticated user being added as reviewer issue- Bug-4929

Review Request #11980 — Created Jan. 22, 2022 and updated April 7, 2022, 12:27 a.m.

Information

Owner

sheenaNg

Repository

Review Board

Branch

release-4.0.x

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

Currently, non-existing username is allowed to be added as a reviewer in Review Board review request even though the username does not exist in the HTTP Digest .htpasswd file. 

The fix involves creating a new helper function that checks if the username is found in the .htpasswd file and have both the authenticate and get_or_create_user functions call this helper function. It will not create a user if the username is not found, and hence it will not allow non-existing username to be added as a reviewer.

Testing Done

Added a test suite for the HTTPDigestBackend class that covers the three methods in the class.
As for manual checking, I attached a couple of files to show the output on the review request.

The first screenshot shows error when adding non-exsiting user as a reviewer. 

The second screenshot shows that an existing user is added as a reviewer successfully. 

The third screenshot shows all the users I have on the list.

Commits

Summary	ID	Author
Fixed HTTP Digest backend unauthenticated user being added as reviewer issue	d3f72f29b4240371814453c4871c6b4222765b9f	sng06
Removed commented-out code	e94f5aaafab924bb77e19a0d9e5fe7dbd040698f	sng06
Fixed styling issues	22357e51b94a85c0140cc1deb396a0d7451e21d4	sng06
Changed the argument name to match what is used in the method definition	ee394285fea704e661d3bf31d5c6e46cfc27c7d7	sng06
Changed method name	f9b8b3d225be7264488ef766d7a05f98545510ac	sng06
Added docstrings to htpasswd_get_user method	9090a5b60a84f135d00d8dfc6528025c6daae068	sng06
Deleted unecessary text in docstrings	53eb03ab994f835fc785b4247d3e1ffbcd3488de	sng06
Added unit test suite for HTTPDigestBackend class	e0266bb57abd53ec55051eb42ab936c478b2d94e	sng06
Fixed over-indented linting issue	48f095c6203d7c074de8ac61498efb922be7c1d0	sng06
Addressed code review feedback	b7239042929242c264824f95dfd3cdcdbc1eafd9	sng06
Changed the htpassword_user_info type to dict in the method parameter	12be423a6edd6f4869f4fbf748dc7d317137bf84	sng06

Files

Issues

Description	From	Last Updated
E265 block comment should start with '# '	reviewbot	Jan. 22, 2022, 1:46 p.m.
W293 blank line contains whitespace	reviewbot	Jan. 22, 2022, 1:46 p.m.
E501 line too long (92 > 79 characters)	reviewbot	Jan. 22, 2022, 1:46 p.m.
E501 line too long (103 > 79 characters)	reviewbot	Jan. 22, 2022, 1:46 p.m.
W293 blank line contains whitespace	reviewbot	Jan. 22, 2022, 1:46 p.m.
E501 line too long (80 > 79 characters)	reviewbot	Jan. 22, 2022, 1:46 p.m.
E501 line too long (84 > 79 characters)	reviewbot	Jan. 22, 2022, 1:46 p.m.
E501 line too long (90 > 79 characters)	reviewbot	Jan. 22, 2022, 1:46 p.m.
W293 blank line contains whitespace	reviewbot	Jan. 22, 2022, 1:46 p.m.
W293 blank line contains whitespace	reviewbot	Jan. 22, 2022, 1:46 p.m.
W391 blank line at end of file	reviewbot	Jan. 22, 2022, 1:46 p.m.
W293 blank line contains whitespace	reviewbot	Jan. 22, 2022, 5:53 p.m.
E501 line too long (92 > 79 characters)	reviewbot	Jan. 22, 2022, 5:53 p.m.
E501 line too long (103 > 79 characters)	reviewbot	Jan. 22, 2022, 5:53 p.m.
W293 blank line contains whitespace	reviewbot	Jan. 22, 2022, 5:53 p.m.
E501 line too long (84 > 79 characters)	reviewbot	Jan. 22, 2022, 5:53 p.m.
E501 line too long (90 > 79 characters)	reviewbot	Jan. 22, 2022, 5:53 p.m.
W293 blank line contains whitespace	reviewbot	Jan. 22, 2022, 5:53 p.m.
W293 blank line contains whitespace	reviewbot	Jan. 22, 2022, 5:53 p.m.
W391 blank line at end of file	reviewbot	Jan. 22, 2022, 5:51 p.m.
We need to use the same name for the argument as what's used in the method definition itself (htpassword_user_info).	mike_conley	Jan. 23, 2022, 11:06 a.m.
E127 continuation line over-indented for visual indent	reviewbot	Jan. 26, 2022, 10:42 p.m.
E127 continuation line over-indented for visual indent	reviewbot	Jan. 26, 2022, 10:42 p.m.
E127 continuation line over-indented for visual indent	reviewbot	Jan. 26, 2022, 10:42 p.m.
We need to add an entry for the new htpassword_user_info arg.	david	April 7, 2022, 12:26 a.m.
This should be shortened so it can fit in one line. If you need additional detail, add another paragraph.	david	April 7, 2022, 12:26 a.m.
Since this is unused, let's just get rid of it. It's needed in the get_or_create_user method because that's the API …	david	April 7, 2022, 12:26 a.m.
dictionary -> dict	david	April 7, 2022, 12:26 a.m.
Please put full-module imports above from imports within the section.	david	April 7, 2022, 12:26 a.m.
When we have to wrap the docstring for test cases, please put the final """ on its own line.	david	April 7, 2022, 12:26 a.m.
These can be condensed: self.spy_on(self.backend._htpasswd_get_user, call_fake=lambda obj: _user_info) self.spy_on(self.backend.get_or_create_user, call_fake=lambda obj, username, request: \ User(username=username))	david	April 7, 2022, 12:27 a.m.
When we have to wrap the docstring for test cases, please put the final """ on its own line.	david	April 7, 2022, 12:27 a.m.
Similar thing here as above.	david	April 7, 2022, 12:27 a.m.
These can use lambdas too.	david	April 7, 2022, 12:27 a.m.
These can use lambdas too	david	April 7, 2022, 12:27 a.m.
If you create this with NamedTemporaryFile(mode='w'), you don't need to mark these as bytestrings.	david	April 7, 2022, 12:27 a.m.
Same here.	david	April 7, 2022, 12:27 a.m.

flake8 failed.

JSHint passed.

flake8

reviewboard/accounts/backends/http_digest.py (Diff revision 1)
The issue has been dropped. Show all issues
```
E265 block comment should start with '# '
```
reviewboard/accounts/backends/http_digest.py (Diff revision 1)
The issue has been dropped. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/accounts/backends/http_digest.py (Diff revision 1)
The issue has been dropped. Show all issues
```
E501 line too long (92 > 79 characters)
```
reviewboard/accounts/backends/http_digest.py (Diff revision 1)
The issue has been dropped. Show all issues
```
E501 line too long (103 > 79 characters)
```
reviewboard/accounts/backends/http_digest.py (Diff revision 1)
The issue has been dropped. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/accounts/backends/http_digest.py (Diff revision 1)
The issue has been dropped. Show all issues
```
E501 line too long (80 > 79 characters)
```
reviewboard/accounts/backends/http_digest.py (Diff revision 1)
The issue has been dropped. Show all issues
```
E501 line too long (84 > 79 characters)
```
reviewboard/accounts/backends/http_digest.py (Diff revision 1)
The issue has been dropped. Show all issues
```
E501 line too long (90 > 79 characters)
```
reviewboard/accounts/backends/http_digest.py (Diff revision 1)
The issue has been dropped. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/accounts/backends/http_digest.py (Diff revision 1)
The issue has been dropped. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/accounts/backends/http_digest.py (Diff revision 1)
The issue has been dropped. Show all issues
```
W391 blank line at end of file
```

Commits:

Summary	ID	Author
Fixed HTTP Digest backend unauthenticated user being added as reviewer issue	d13eacca0a862c1e23fa36110fc74e98c33b7978	sng06
Fixed HTTP Digest backend unauthenticated user being added as reviewer issue	d13eacca0a862c1e23fa36110fc74e98c33b7978	sng06
Removed commented-out code	56e8ccaaa2f9c367f53fd7349c3c16e7bb9f9b53	sng06

Diff:

Revision 2 (+113 -71)

Show changes

reviewboard/accounts/backends/http_digest.py

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

reviewboard/accounts/backends/http_digest.py (Diff revision 2)
The issue has been dropped. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/accounts/backends/http_digest.py (Diff revision 2)
The issue has been dropped. Show all issues
```
E501 line too long (92 > 79 characters)
```
reviewboard/accounts/backends/http_digest.py (Diff revision 2)
The issue has been dropped. Show all issues
```
E501 line too long (103 > 79 characters)
```
reviewboard/accounts/backends/http_digest.py (Diff revision 2)
The issue has been dropped. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/accounts/backends/http_digest.py (Diff revision 2)
The issue has been dropped. Show all issues
```
E501 line too long (84 > 79 characters)
```
reviewboard/accounts/backends/http_digest.py (Diff revision 2)
The issue has been dropped. Show all issues
```
E501 line too long (90 > 79 characters)
```
reviewboard/accounts/backends/http_digest.py (Diff revision 2)
The issue has been dropped. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/accounts/backends/http_digest.py (Diff revision 2)
The issue has been dropped. Show all issues
```
W293 blank line contains whitespace
```
reviewboard/accounts/backends/http_digest.py (Diff revision 2)
The issue has been resolved. Show all issues
```
W391 blank line at end of file
```

Commits:

Summary	ID	Author
Fixed HTTP Digest backend unauthenticated user being added as reviewer issue	d13eacca0a862c1e23fa36110fc74e98c33b7978	sng06
Removed commented-out code	56e8ccaaa2f9c367f53fd7349c3c16e7bb9f9b53	sng06
Fixed HTTP Digest backend unauthenticated user being added as reviewer issue	d13eacca0a862c1e23fa36110fc74e98c33b7978	sng06
Removed commented-out code	56e8ccaaa2f9c367f53fd7349c3c16e7bb9f9b53	sng06
Fixed styling issues	e2ea3e38d38548a6f869ef16a48466dac8cfd862	sng06

Diff:

Revision 3 (+128 -78)

Show changes

reviewboard/accounts/backends/http_digest.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Thanks, Sheena! Looking good so far - looking forward to seeing those tests. Just one note below.

reviewboard/accounts/backends/http_digest.py (Diff revision 3)

The issue has been resolved. Show all issues

We need to use the same name for the argument as what's used in the method definition itself (htpassword_user_info).

Commits:

Summary	ID	Author
Fixed HTTP Digest backend unauthenticated user being added as reviewer issue	d13eacca0a862c1e23fa36110fc74e98c33b7978	sng06
Removed commented-out code	56e8ccaaa2f9c367f53fd7349c3c16e7bb9f9b53	sng06
Fixed styling issues	e2ea3e38d38548a6f869ef16a48466dac8cfd862	sng06
Fixed HTTP Digest backend unauthenticated user being added as reviewer issue	d13eacca0a862c1e23fa36110fc74e98c33b7978	sng06
Removed commented-out code	56e8ccaaa2f9c367f53fd7349c3c16e7bb9f9b53	sng06
Fixed styling issues	e2ea3e38d38548a6f869ef16a48466dac8cfd862	sng06
Changed the argument name to match what is used in the method definition	7cb6fc6f995d4609066500d96e293487c5eda816	sng06

Diff:

Revision 4 (+129 -79)

Show changes

reviewboard/accounts/backends/http_digest.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Change Summary:

Changed the method name, added docstrings to htpasswd_get_user method, and added test suite for the HTTPDigestBackend class.

Testing Done:

~		Will be adding some unit tests after the code sprint.
	~	Added a test suite for the HTTPDigestBackend class that covers the three methods in the class.

		As for manual checking, I attached a couple of files to show the output on the review request.
		The first screenshot shows error when adding non-exsiting user as a reviewer.
		The second screenshot shows that an existing user is added as a reviewer successfully.
		The third screenshot shows all the users I have on the list.

Commits:

Summary	ID	Author
Fixed HTTP Digest backend unauthenticated user being added as reviewer issue	d13eacca0a862c1e23fa36110fc74e98c33b7978	sng06
Removed commented-out code	56e8ccaaa2f9c367f53fd7349c3c16e7bb9f9b53	sng06
Fixed styling issues	e2ea3e38d38548a6f869ef16a48466dac8cfd862	sng06
Changed the argument name to match what is used in the method definition	7cb6fc6f995d4609066500d96e293487c5eda816	sng06
Fixed HTTP Digest backend unauthenticated user being added as reviewer issue	d13eacca0a862c1e23fa36110fc74e98c33b7978	sng06
Removed commented-out code	56e8ccaaa2f9c367f53fd7349c3c16e7bb9f9b53	sng06
Fixed styling issues	e2ea3e38d38548a6f869ef16a48466dac8cfd862	sng06
Changed the argument name to match what is used in the method definition	7cb6fc6f995d4609066500d96e293487c5eda816	sng06
Changed method name	a076a4b7ac8d58361c7ab1c96886a2cc59cad47d	sng06
Added docstrings to htpasswd_get_user method	01fa738db6f34ec5b875a66896be9866d2e982c4	sng06
Deleted unecessary text in docstrings	b9c005b75065fabe4c4ee4133c603a76f376a240	sng06
Added unit test suite for HTTPDigestBackend class	381c2575d9abaeeab18a9e873b6fb4bca912bf2f	sng06

Diff:

Revision 5 (+617 -93)

Show changes

	reviewboard/accounts/backends/http_digest.py
	reviewboard/accounts/tests/test_http_digest_auth_backend.py

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

reviewboard/accounts/backends/http_digest.py (Diff revision 5)
The issue has been resolved. Show all issues
```
E127 continuation line over-indented for visual indent
```
reviewboard/accounts/tests/test_http_digest_auth_backend.py (Diff revision 5)
The issue has been resolved. Show all issues
```
E127 continuation line over-indented for visual indent
```
reviewboard/accounts/tests/test_http_digest_auth_backend.py (Diff revision 5)
The issue has been resolved. Show all issues
```
E127 continuation line over-indented for visual indent
```

Change Summary:

Fixed linting issues

Summary:

[WIP] Fixed HTTP Digest backend unauthenticated user being added as reviewer issue- Bug-4929

Fixed HTTP Digest backend unauthenticated user being added as reviewer issue- Bug-4929

Commits:

Summary	ID	Author
Fixed HTTP Digest backend unauthenticated user being added as reviewer issue	d13eacca0a862c1e23fa36110fc74e98c33b7978	sng06
Removed commented-out code	56e8ccaaa2f9c367f53fd7349c3c16e7bb9f9b53	sng06
Fixed styling issues	e2ea3e38d38548a6f869ef16a48466dac8cfd862	sng06
Changed the argument name to match what is used in the method definition	7cb6fc6f995d4609066500d96e293487c5eda816	sng06
Changed method name	a076a4b7ac8d58361c7ab1c96886a2cc59cad47d	sng06
Added docstrings to htpasswd_get_user method	01fa738db6f34ec5b875a66896be9866d2e982c4	sng06
Deleted unecessary text in docstrings	b9c005b75065fabe4c4ee4133c603a76f376a240	sng06
Added unit test suite for HTTPDigestBackend class	381c2575d9abaeeab18a9e873b6fb4bca912bf2f	sng06
Fixed HTTP Digest backend unauthenticated user being added as reviewer issue	d13eacca0a862c1e23fa36110fc74e98c33b7978	sng06
Removed commented-out code	56e8ccaaa2f9c367f53fd7349c3c16e7bb9f9b53	sng06
Fixed styling issues	e2ea3e38d38548a6f869ef16a48466dac8cfd862	sng06
Changed the argument name to match what is used in the method definition	7cb6fc6f995d4609066500d96e293487c5eda816	sng06
Changed method name	a076a4b7ac8d58361c7ab1c96886a2cc59cad47d	sng06
Added docstrings to htpasswd_get_user method	01fa738db6f34ec5b875a66896be9866d2e982c4	sng06
Deleted unecessary text in docstrings	b9c005b75065fabe4c4ee4133c603a76f376a240	sng06
Added unit test suite for HTTPDigestBackend class	381c2575d9abaeeab18a9e873b6fb4bca912bf2f	sng06
Fixed over-indented linting issue	ba6b5d054bde6c084726791dd29abb96529fb716	sng06

Diff:

Revision 6 (+620 -96)

Show changes

	reviewboard/accounts/backends/http_digest.py
	reviewboard/accounts/tests/test_http_digest_auth_backend.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/accounts/backends/http_digest.py (Diff revision 6)
The issue has been resolved. Show all issues
```
We need to add an entry for the new htpassword_user_info arg.
```

reviewboard/accounts/backends/http_digest.py (Diff revision 6)

The issue has been resolved. Show all issues

This should be shortened so it can fit in one line. If you need additional detail, add another paragraph.

reviewboard/accounts/backends/http_digest.py (Diff revision 6)

The issue has been resolved. Show all issues

Since this is unused, let's just get rid of it. It's needed in the get_or_create_user method because that's the API contract that this class is fulfilling, but here this is just a helper method.

reviewboard/accounts/backends/http_digest.py (Diff revision 6)
The issue has been resolved. Show all issues
```
dictionary -> dict
```
reviewboard/accounts/tests/test_http_digest_auth_backend.py (Diff revision 6)
The issue has been resolved. Show all issues
```
Please put full-module imports above from imports within the section.
```
reviewboard/accounts/tests/test_http_digest_auth_backend.py (Diff revision 6)
The issue has been resolved. Show all issues
```
When we have to wrap the docstring for test cases, please put the final """ on its own line.
```

reviewboard/accounts/tests/test_http_digest_auth_backend.py (Diff revision 6)

The issue has been resolved. Show all issues

These can be condensed:

self.spy_on(self.backend._htpasswd_get_user,
            call_fake=lambda obj: _user_info)
self.spy_on(self.backend.get_or_create_user,
            call_fake=lambda obj, username, request: \
                User(username=username))

reviewboard/accounts/tests/test_http_digest_auth_backend.py (Diff revision 6)
The issue has been resolved. Show all issues
```
When we have to wrap the docstring for test cases, please put the final """ on its own line.
```

reviewboard/accounts/tests/test_http_digest_auth_backend.py (Diff revision 6)

The issue has been resolved. Show all issues

Similar thing here as above.

reviewboard/accounts/tests/test_http_digest_auth_backend.py (Diff revision 6)
The issue has been resolved. Show all issues
```
These can use lambdas too.
```
reviewboard/accounts/tests/test_http_digest_auth_backend.py (Diff revision 6)
The issue has been resolved. Show all issues
```
These can use lambdas too
```
reviewboard/accounts/tests/test_http_digest_auth_backend.py (Diff revision 6)
The issue has been resolved. Show all issues
```
If you create this with NamedTemporaryFile(mode='w'), you don't need to mark these as bytestrings.
```
reviewboard/accounts/tests/test_http_digest_auth_backend.py (Diff revision 6)
The issue has been resolved. Show all issues
```
Same here.
```

Change Summary:

Addressed code review feedback.

Commits:

Summary	ID	Author
Fixed HTTP Digest backend unauthenticated user being added as reviewer issue	d13eacca0a862c1e23fa36110fc74e98c33b7978	sng06
Removed commented-out code	56e8ccaaa2f9c367f53fd7349c3c16e7bb9f9b53	sng06
Fixed styling issues	e2ea3e38d38548a6f869ef16a48466dac8cfd862	sng06
Changed the argument name to match what is used in the method definition	7cb6fc6f995d4609066500d96e293487c5eda816	sng06
Changed method name	a076a4b7ac8d58361c7ab1c96886a2cc59cad47d	sng06
Added docstrings to htpasswd_get_user method	01fa738db6f34ec5b875a66896be9866d2e982c4	sng06
Deleted unecessary text in docstrings	b9c005b75065fabe4c4ee4133c603a76f376a240	sng06
Added unit test suite for HTTPDigestBackend class	381c2575d9abaeeab18a9e873b6fb4bca912bf2f	sng06
Fixed over-indented linting issue	ba6b5d054bde6c084726791dd29abb96529fb716	sng06
Fixed HTTP Digest backend unauthenticated user being added as reviewer issue	d3f72f29b4240371814453c4871c6b4222765b9f	sng06
Removed commented-out code	e94f5aaafab924bb77e19a0d9e5fe7dbd040698f	sng06
Fixed styling issues	22357e51b94a85c0140cc1deb396a0d7451e21d4	sng06
Changed the argument name to match what is used in the method definition	ee394285fea704e661d3bf31d5c6e46cfc27c7d7	sng06
Changed method name	f9b8b3d225be7264488ef766d7a05f98545510ac	sng06
Added docstrings to htpasswd_get_user method	9090a5b60a84f135d00d8dfc6528025c6daae068	sng06
Deleted unecessary text in docstrings	53eb03ab994f835fc785b4247d3e1ffbcd3488de	sng06
Added unit test suite for HTTPDigestBackend class	e0266bb57abd53ec55051eb42ab936c478b2d94e	sng06
Fixed over-indented linting issue	48f095c6203d7c074de8ac61498efb922be7c1d0	sng06
Addressed code review feedback	b7239042929242c264824f95dfd3cdcdbc1eafd9	sng06
Changed the htpassword_user_info type to dict in the method parameter	12be423a6edd6f4869f4fbf748dc7d317137bf84	sng06

Diff:

Revision 7 (+640 -174)

Show changes

	reviewboard/accounts/backends/http_digest.py
	reviewboard/accounts/tests/test_http_digest_auth_backend.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.