Summary

Added sandboxing to AuthBackend

Review Request #5736 — Created April 24, 2014 and discarded Oct. 16, 2014, 2:40 p.m.

Information

Owner

edwinzg

Repository

Review Board

Branch

master

Bugs

Depends On

Reviewers

Groups

reviewboard, students

People

Description

Added sandboxing to AuthBackend
Writing unit tests for the sandboxing for get_queryset and search_queryset

Testing Done

Unit testing for sandboxing get_queryset and search_queryset for AuthBackend

Issues

Description	From	Last Updated
This should be pulled out into a separate change.	david	May 16, 2014, 3:21 p.m.
You could use a lambda expression here, rather than defining a new method: self.spy_on(get_enabled_auth_backends, call_fake=lambda: [AuthTestQueryUsersBackend()])	david	May 17, 2014, 10:13 a.m.
These should also use lambdas.	david	May 17, 2014, 10:13 a.m.
local variable 'hook' is assigned to but never used	reviewbot	May 18, 2014, 8:08 a.m.
Col: 1 W391 blank line at end of file	reviewbot	May 17, 2014, 2:07 p.m.
local variable 'hook' is assigned to but never used	reviewbot	May 18, 2014, 8:08 a.m.

The registration code itself isn't something that I think needs to be sandboxed, since it doesn't run any third-party code. Instead, we want to sandox calls into the auth backends. You can search for get_enabled_auth_backends in the code to find where we iterate over auth backends and call methods on them.

Summary:

Added sandboxing to register_auth_backend

[WIP] Added sandboxing to AuthBackend

Description:

~		Added sandboxing to register_auth_backend
	~	Added sandboxing to AuthBackend
-
-		Not sure how to raise the KeyError that register_auth_backend raises.
-
-		Also not sure how to test it

Diff:

Revision 2 (+16 -2)

Show changes

	reviewboard/extensions/tests.py
	reviewboard/webapi/resources/user.py

This is much better. I'm looking forward to seeing the full change including unit tests.

Diff:

Revision 3 (+36 -4)

Show changes

	reviewboard/extensions/tests.py
	reviewboard/webapi/resources/user.py

Diff:

Revision 4 (+61 -7)

Show changes

	reviewboard/extensions/tests.py
	reviewboard/extensions/templatetags/rb_extensions.py
	reviewboard/webapi/resources/user.py

```
What's left to get this out of WIP?
```

Description:: ~
Added sandboxing to AuthBackend
~
Added sandboxing to AuthBackend
+ Writing unit tests for the sandboxing for get_queryset and search_queryset

Summary:

[WIP] Added sandboxing to AuthBackend

Added sandboxing to AuthBackend

Testing Done:

Unit testing for sandboxing get_queryset and search_queryset for AuthBackend

Diff:

Revision 5 (+83 -8)

Show changes

	reviewboard/extensions/tests.py
	reviewboard/extensions/templatetags/rb_extensions.py
	reviewboard/webapi/resources/user.py

Fix it!

reviewboard/extensions/templatetags/rb_extensions.py (Diff revision 5)

An issue was opened. Show all issues

This should be pulled out into a separate change.

edwinzg May 17, 2014, 10:02 a.m.

It was, but I think when I merged with master it messed this part up.

reviewboard/extensions/tests.py (Diff revision 5)

The issue has been resolved. Show all issues

You could use a lambda expression here, rather than defining a new method:

self.spy_on(get_enabled_auth_backends,
            call_fake=lambda: [AuthTestQueryUsersBackend()])

reviewboard/extensions/tests.py (Diff revision 5)
The issue has been resolved. Show all issues
```
These should also use lambdas.
```

Diff:

Revision 6 (+72 -8)

Show changes

	reviewboard/extensions/tests.py
	reviewboard/extensions/templatetags/rb_extensions.py
	reviewboard/webapi/resources/user.py

This is a review from Review Bot.
  Tool: PEP8 Style Checker
  Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/webapi/resources/user.py
    reviewboard/extensions/tests.py
  Ignored Files:

reviewboard/extensions/tests.py (Diff revision 6)
The issue has been resolved. Show all issues
```
Col: 1
 W391 blank line at end of file
```

This is a review from Review Bot.
  Tool: Pyflakes
  Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/webapi/resources/user.py
    reviewboard/extensions/tests.py
  Ignored Files:

reviewboard/extensions/tests.py (Diff revision 6)
The issue has been dropped. Show all issues
```
 local variable 'hook' is assigned to but never used
```

Diff:

Revision 7 (+71 -8)

Show changes

	reviewboard/extensions/tests.py
	reviewboard/extensions/templatetags/rb_extensions.py
	reviewboard/webapi/resources/user.py

This is a review from Review Bot.
  Tool: PEP8 Style Checker
  Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/webapi/resources/user.py
    reviewboard/extensions/tests.py
  Ignored Files:

This is a review from Review Bot.
  Tool: Pyflakes
  Processed Files:
    reviewboard/extensions/templatetags/rb_extensions.py
    reviewboard/webapi/resources/user.py
    reviewboard/extensions/tests.py
  Ignored Files:

reviewboard/extensions/tests.py (Diff revision 7)
The issue has been dropped. Show all issues
```
 local variable 'hook' is assigned to but never used
```

Status:: Discarded

~		Added sandboxing to AuthBackend
	~	Added sandboxing to AuthBackend
	+	Writing unit tests for the sandboxing for get_queryset and search_queryset