Summary

Add support for using API tokens for authentication.

Review Request #6079 — Created July 8, 2014 and submitted July 22, 2014, 6:27 p.m.

Information

Owner

chipx86

Repository

Review Board

Branch

master

Bugs

Depends On

Blocks

6080

Commit

df7d622...

Reviewers

Groups

reviewboard

People

Description

This introduces new auth and webapi auth backends for performing API
requests using an API token. Users can have zero or more API tokens
linked to their account, and any will be valid for authentication. The
goal is to allow tokens for different access levels (read/write,
read-only) and, down the road, scopes.

Clients can use these when talking to the API by passing
Authorization: token <token_value> in the request. This will
authenticate the owner of that token for the API requests.

Tokens cannot be used to authenticate through the website.

For now, tokens must be created through the admin UI. Further changes
are coming to make the usage of tokens more practical.

Testing Done

I tested with 2 LocalSites and the global site.
I created three auth tokens: One unbound to a LocalSite, and one bound to each LocalSite.
Using curl, I attempted to access /api/session/ on the global site and on each LocalSite using each token. Results:

Global token could reach the global /api/session/ and that of each LocalSite.
LocalSite token 1 could only reach /s/site1/api/session/, and got a Permission Denied elsewhere.
Same behavior for LocalSite token 2, except it could only reach /s/site2/api/session/.

Specifying an invalid token gave me an authentication error.
Unit tests pass (except that some on master haven't been updated for the function renames, but those are irrelevant to these changes).

Issues

Description	From	Last Updated
'from settings_local import *' used; unable to detect undefined names	reviewbot	July 10, 2014, 1:40 a.m.
'PIPELINE_CSS' imported but unused	reviewbot	July 10, 2014, 1:40 a.m.
'PIPELINE_JS' imported but unused	reviewbot	July 10, 2014, 1:40 a.m.
'from settings_local import *' used; unable to detect undefined names	reviewbot	July 10, 2014, 1:40 a.m.
'PIPELINE_CSS' imported but unused	reviewbot	July 10, 2014, 1:40 a.m.
'PIPELINE_JS' imported but unused	reviewbot	July 10, 2014, 1:40 a.m.
The second line here isn't indented properly (it should be inside the hashlib.sha1() call). You can also get rid of …	david	July 10, 2014, 1:40 a.m.
This isn't inside an exception handler so exc_info doesn't mean anything.	david	July 10, 2014, 1:40 a.m.
Is this meant to be returned via the API, or shown in the UI? If the former, we don't usually …	david	July 10, 2014, 1:49 a.m.
This could then just be TokenAuthBackend, to make it clear that it's not specifically an API backend.	david	July 22, 2014, 6:12 p.m.
So I just noticed the inconsistency in the names here. How about naming the second one WebAPITokenAuthBackend?	david	July 22, 2014, 6:12 p.m.
'from settings_local import *' used; unable to detect undefined names	reviewbot	July 10, 2014, 1:52 a.m.
'PIPELINE_CSS' imported but unused	reviewbot	July 10, 2014, 1:52 a.m.
'PIPELINE_JS' imported but unused	reviewbot	July 10, 2014, 1:52 a.m.
'django_reset' imported but unused	reviewbot	July 22, 2014, 6:15 p.m.
'from settings_local import *' used; unable to detect undefined names	reviewbot	July 22, 2014, 6:15 p.m.
'PIPELINE_JS' imported but unused	reviewbot	July 22, 2014, 6:15 p.m.
'PIPELINE_CSS' imported but unused	reviewbot	July 22, 2014, 6:15 p.m.
undefined name 'APITokenWebAPIAuthBackend'	reviewbot	July 22, 2014, 6:15 p.m.
'django_reset' imported but unused	reviewbot	July 22, 2014, 6:17 p.m.
'from settings_local import *' used; unable to detect undefined names	reviewbot	July 22, 2014, 6:16 p.m.
'PIPELINE_JS' imported but unused	reviewbot	July 22, 2014, 6:16 p.m.
'PIPELINE_CSS' imported but unused	reviewbot	July 22, 2014, 6:17 p.m.

Change Summary:


The WebAPIToken ID is now stored in the session, and the token re-fetched later, instead of storing the Local Site ID itself in the session. That way, changing the Local Site associated with the token will affect existing sessions. We'll also be needing that token in other changes.
Deleting a token now invalidates existing sessions.

Description:

		This introduces new auth and webapi auth backends for performing API
		requests using an API token. Users can have zero or more API tokens
		linked to their account, and any will be valid for authentication. The
		goal is to allow tokens for different access levels (read/write,
		read-only) and, down the road, scopes.

		Clients can use these when talking to the API by passing
~		`Authorization: token <token_value\>` in the request. This will
	~	`Authorization: token <token_value>` in the request. This will
		authenticate the owner of that token for the API requests.

		Tokens cannot be used to authenticate through the website.

		For now, tokens must be created through the admin UI. Further changes
		are coming to make the usage of tokens more practical.

Commit:

-	a452adc94fe65196c255c3a2cf1b2098f9f02707
+	ebc2333e4220f97e6b5cdf3bb26ed1a27117e32d

Diff:

Revision 2 (+271 -9)

Show changes

	reviewboard/settings.py
	reviewboard/admin/siteconfig.py
	reviewboard/webapi/admin.py
	reviewboard/webapi/auth_backends.py
	reviewboard/webapi/base.py
	reviewboard/webapi/decorators.py
	reviewboard/webapi/errors.py
	reviewboard/webapi/managers.py
	1 more

Tool: PEP8 Style Checker
Processed Files:
    reviewboard/admin/siteconfig.py
    reviewboard/webapi/base.py
    reviewboard/settings.py
    reviewboard/webapi/models.py
    reviewboard/webapi/errors.py
    reviewboard/webapi/auth_backends.py
    reviewboard/webapi/admin.py
    reviewboard/webapi/decorators.py
    reviewboard/webapi/managers.py



Tool: Pyflakes
Processed Files:
    reviewboard/admin/siteconfig.py
    reviewboard/webapi/base.py
    reviewboard/settings.py
    reviewboard/webapi/models.py
    reviewboard/webapi/errors.py
    reviewboard/webapi/auth_backends.py
    reviewboard/webapi/admin.py
    reviewboard/webapi/decorators.py
    reviewboard/webapi/managers.py

reviewboard/settings.py (Diff revision 2)
The issue has been dropped. Show all issues
```
 'from settings_local import *' used; unable to detect undefined names
```
reviewboard/settings.py (Diff revision 2)
The issue has been dropped. Show all issues
```
 'PIPELINE_CSS' imported but unused
```
reviewboard/settings.py (Diff revision 2)
The issue has been dropped. Show all issues
```
 'PIPELINE_JS' imported but unused
```

Tool: Pyflakes
Processed Files:
    reviewboard/admin/siteconfig.py
    reviewboard/webapi/models.py
    reviewboard/settings.py
    reviewboard/webapi/errors.py
    reviewboard/webapi/auth_backends.py
    reviewboard/webapi/admin.py
    reviewboard/webapi/decorators.py
    reviewboard/webapi/managers.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/admin/siteconfig.py
    reviewboard/webapi/models.py
    reviewboard/settings.py
    reviewboard/webapi/errors.py
    reviewboard/webapi/auth_backends.py
    reviewboard/webapi/admin.py
    reviewboard/webapi/decorators.py
    reviewboard/webapi/managers.py

reviewboard/settings.py (Diff revision 1)
The issue has been dropped. Show all issues
```
 'from settings_local import *' used; unable to detect undefined names
```
reviewboard/settings.py (Diff revision 1)
The issue has been dropped. Show all issues
```
 'PIPELINE_CSS' imported but unused
```
reviewboard/settings.py (Diff revision 1)
The issue has been dropped. Show all issues
```
 'PIPELINE_JS' imported but unused
```

reviewboard/webapi/managers.py (Diff revision 2)

The issue has been resolved. Show all issues

The second line here isn't indented properly (it should be inside the hashlib.sha1() call). You can also get rid of the outer parens.

reviewboard/webapi/managers.py (Diff revision 2)
The issue has been resolved. Show all issues
```
This isn't inside an exception handler so exc_info doesn't mean anything.
```

reviewboard/webapi/managers.py (Diff revision 2)

The issue has been dropped. Show all issues

Is this meant to be returned via the API, or shown in the UI? If the former, we don't usually translate API strings, right?

chipx86 July 10, 2014, 1:41 a.m.

It's used for both. We don't usually translate, but the strings are for display purposes and not for parsing. I'm pretty sure we have other errors that can be returned by the API that are localized.

chipx86 July 10, 2014, 1:49 a.m.

Yep, definitely errors used elsewhere. DiffTooBigError's string is localized and returned. Same with EmptyDiffError, ShortSHA1Error. Probably others.

We may just want to localize for all errors, really.

Change Summary:


Fixed indentation when generating a token.
Removed a exc_info=1 on a logging call where it wasn't needed.

Commit:

-	ebc2333e4220f97e6b5cdf3bb26ed1a27117e32d
+	4a9e5f0c517be8f200c51d8ffe22bb8eb13273b1

Diff:

Revision 3 (+270 -9)

Show changes

	reviewboard/settings.py
	reviewboard/admin/siteconfig.py
	reviewboard/webapi/admin.py
	reviewboard/webapi/auth_backends.py
	reviewboard/webapi/base.py
	reviewboard/webapi/decorators.py
	reviewboard/webapi/errors.py
	reviewboard/webapi/managers.py
	1 more

Tool: PEP8 Style Checker
Processed Files:
    reviewboard/admin/siteconfig.py
    reviewboard/webapi/base.py
    reviewboard/settings.py
    reviewboard/webapi/models.py
    reviewboard/webapi/errors.py
    reviewboard/webapi/auth_backends.py
    reviewboard/webapi/admin.py
    reviewboard/webapi/decorators.py
    reviewboard/webapi/managers.py



Tool: Pyflakes
Processed Files:
    reviewboard/admin/siteconfig.py
    reviewboard/webapi/base.py
    reviewboard/settings.py
    reviewboard/webapi/models.py
    reviewboard/webapi/errors.py
    reviewboard/webapi/auth_backends.py
    reviewboard/webapi/admin.py
    reviewboard/webapi/decorators.py
    reviewboard/webapi/managers.py

reviewboard/settings.py (Diff revision 3)
The issue has been dropped. Show all issues
```
 'from settings_local import *' used; unable to detect undefined names
```
reviewboard/settings.py (Diff revision 3)
The issue has been dropped. Show all issues
```
 'PIPELINE_CSS' imported but unused
```
reviewboard/settings.py (Diff revision 3)
The issue has been dropped. Show all issues
```
 'PIPELINE_JS' imported but unused
```

reviewboard/admin/siteconfig.py (Diff revision 3)

The issue has been resolved. Show all issues

This could then just be TokenAuthBackend, to make it clear that it's not specifically an API backend.

reviewboard/settings.py (Diff revision 3)

The issue has been resolved. Show all issues

So I just noticed the inconsistency in the names here. How about naming the second one WebAPITokenAuthBackend?

chipx86 July 10, 2014, 12:35 p.m.

The reason for that naming is because we have a WebAPIAuthBackend in here, and a standard AuthBackend in that module.

david July 10, 2014, 12:39 p.m.

Yeah, I'm saying that I'd prefer it as 'TokenAuthBackend' and 'WebAPITokenAuthBackend'

Change Summary:

Renamed the classes.

Commit:

-	4a9e5f0c517be8f200c51d8ffe22bb8eb13273b1
+	f3eef2a895f6613656245cdef25056791e00ef3e

Diff:

Revision 4 (+270 -9)

Show changes

	reviewboard/settings.py
	reviewboard/admin/siteconfig.py
	reviewboard/webapi/admin.py
	reviewboard/webapi/auth_backends.py
	reviewboard/webapi/base.py
	reviewboard/webapi/decorators.py
	reviewboard/webapi/errors.py
	reviewboard/webapi/managers.py
	1 more

Tool: PEP8 Style Checker
Processed Files:
    reviewboard/admin/siteconfig.py
    reviewboard/webapi/base.py
    reviewboard/settings.py
    reviewboard/webapi/models.py
    reviewboard/webapi/errors.py
    reviewboard/webapi/auth_backends.py
    reviewboard/webapi/admin.py
    reviewboard/webapi/decorators.py
    reviewboard/webapi/managers.py



Tool: Pyflakes
Processed Files:
    reviewboard/admin/siteconfig.py
    reviewboard/webapi/base.py
    reviewboard/settings.py
    reviewboard/webapi/models.py
    reviewboard/webapi/errors.py
    reviewboard/webapi/auth_backends.py
    reviewboard/webapi/admin.py
    reviewboard/webapi/decorators.py
    reviewboard/webapi/managers.py

reviewboard/settings.py (Diff revision 4)
The issue has been dropped. Show all issues
```
 'django_reset' imported but unused
```
reviewboard/settings.py (Diff revision 4)
The issue has been dropped. Show all issues
```
 'from settings_local import *' used; unable to detect undefined names
```
reviewboard/settings.py (Diff revision 4)
The issue has been dropped. Show all issues
```
 'PIPELINE_JS' imported but unused
```
reviewboard/settings.py (Diff revision 4)
The issue has been dropped. Show all issues
```
 'PIPELINE_CSS' imported but unused
```
reviewboard/webapi/auth_backends.py (Diff revision 4)
The issue has been resolved. Show all issues
```
 undefined name 'APITokenWebAPIAuthBackend'
```

Change Summary:

Fixed the super call.

Commit:

-	f3eef2a895f6613656245cdef25056791e00ef3e
+	df7d622b553da5720e690b4207581306ca26ff71

Diff:

Revision 5 (+270 -9)

Show changes

	reviewboard/settings.py
	reviewboard/admin/siteconfig.py
	reviewboard/webapi/admin.py
	reviewboard/webapi/auth_backends.py
	reviewboard/webapi/base.py
	reviewboard/webapi/decorators.py
	reviewboard/webapi/errors.py
	reviewboard/webapi/managers.py
	1 more

Tool: Pyflakes
Processed Files:
    reviewboard/admin/siteconfig.py
    reviewboard/webapi/base.py
    reviewboard/settings.py
    reviewboard/webapi/models.py
    reviewboard/webapi/errors.py
    reviewboard/webapi/auth_backends.py
    reviewboard/webapi/admin.py
    reviewboard/webapi/decorators.py
    reviewboard/webapi/managers.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/admin/siteconfig.py
    reviewboard/webapi/base.py
    reviewboard/settings.py
    reviewboard/webapi/models.py
    reviewboard/webapi/errors.py
    reviewboard/webapi/auth_backends.py
    reviewboard/webapi/admin.py
    reviewboard/webapi/decorators.py
    reviewboard/webapi/managers.py

reviewboard/settings.py (Diff revision 5)
The issue has been dropped. Show all issues
```
 'django_reset' imported but unused
```
reviewboard/settings.py (Diff revision 5)
The issue has been dropped. Show all issues
```
 'from settings_local import *' used; unable to detect undefined names
```
reviewboard/settings.py (Diff revision 5)
The issue has been dropped. Show all issues
```
 'PIPELINE_JS' imported but unused
```
reviewboard/settings.py (Diff revision 5)
The issue has been dropped. Show all issues
```
 'PIPELINE_CSS' imported but unused
```

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to master (8ef5f85)