Summary

[OAuth2Provider] Add dependencies and urls to provide OAuth2 support for external services

Review Request #7897 — Created Jan. 20, 2016 and discarded Sept. 7, 2017, 8:59 p.m.

Information

Owner

lehoangm

Repository

Review Board

Branch

master

Bugs

Depends On

Blocks

7900

Reviewers

Groups

reviewboard

People

brennie, chipx86, david

Description

Added into list of installed apps package django-tool-kit which provides

end-points and basic logic to manage client applications. All the end-points

for OAuth2 authorization with external services is moved to /oauth2/.

All the tool to manage client applications is a part of preference menu
Added package django-tool-kit into setup.py so that it will get install

with ReviewBoard during setup time. Even though the lastest version is 0.10,

the chosen version is 0.9 in order to be compatible with Django 1.6.
Note: please contact lehoangminh@live.com when you try to land the feature into master. I am more than happy to support the final integration

Testing Done

Manually tested the urls are setup correctly
Reran python setup.py develop

Issues

Description	From	Last Updated
The first import should be: from __future__ import unicode_literals	brennie	Jan. 20, 2016, 10:46 a.m.
Docstrings should be of the form: """Single line summary. Multi-line description. """	brennie	Jan. 20, 2016, 10:47 a.m.
Blank line between these.	brennie	Jan. 20, 2016, 10:48 a.m.
Col: 5 E265 block comment should start with '# '	reviewbot	Jan. 20, 2016, 10:45 a.m.
Col: 9 W292 no newline at end of file	reviewbot	Jan. 20, 2016, 10:45 a.m.
Unnecessary.	brennie	Jan. 20, 2016, 10:48 a.m.
We're only importing one set of views here. I think we should do: from oauth2_provider import views	brennie	Jan. 20, 2016, 10:48 a.m.
"Review Board" "OAuth" Missing a period.	brennie	Jan. 20, 2016, 10:49 a.m.
Single quotes. Same below.	brennie	Jan. 20, 2016, 10:50 a.m.
"OAuth" Also missing a period.	brennie	Jan. 20, 2016, 10:50 a.m.
Use patterns()	brennie	Jan. 20, 2016, 10:51 a.m.
No blank line here.	brennie	Jan. 20, 2016, 10:51 a.m.
Col: 2 W292 no newline at end of file	reviewbot	Jan. 20, 2016, 10:45 a.m.
'django_reset' imported but unused	reviewbot	Jan. 20, 2016, 8:20 a.m.
'from settings_local import *' used; unable to detect undefined names	reviewbot	Jan. 20, 2016, 8:20 a.m.
'PIPELINE_CSS' imported but unused	reviewbot	Jan. 20, 2016, 8:20 a.m.
'PIPELINE_JS' imported but unused	reviewbot	Jan. 20, 2016, 8:20 a.m.
Missing a trailing comma.	brennie	Jan. 20, 2016, 10:51 a.m.
Please keep these imports in alphabetical order.	brennie	Jan. 20, 2016, 10:54 a.m.
Unnecessary comment.	brennie	Jan. 20, 2016, 10:54 a.m.
This isn't the right place for this. account/ URLs should go in reviewboard/accounts/urls.py. There, you can leave off the account/ …	brennie	Jan. 20, 2016, 10:59 a.m.
Col: 34 W292 no newline at end of file	reviewbot	Jan. 20, 2016, 10:46 a.m.
'django_reset' imported but unused	reviewbot	Jan. 20, 2016, 11:13 a.m.
'from settings_local import *' used; unable to detect undefined names	reviewbot	Jan. 20, 2016, 11:13 a.m.
'PIPELINE_CSS' imported but unused	reviewbot	Jan. 20, 2016, 11:13 a.m.
'PIPELINE_JS' imported but unused	reviewbot	Jan. 20, 2016, 11:13 a.m.
Alphabetize these.	brennie	Jan. 21, 2016, 5:44 p.m.
This import doesn't need the as ... clause. When you do from x import y as y it is the …	brennie	Jan. 21, 2016, 5:45 p.m.
url(r'^preferences/oauth2/', include(apps_urlpatterns, namespace='oauth2_provider'))	brennie	Jan. 21, 2016, 5:46 p.m.
Why are we including this with a namespace?	brennie	Jan. 25, 2016, 10:27 p.m.
'django_reset' imported but unused	reviewbot	Jan. 20, 2016, 3:32 p.m.
'from settings_local import *' used; unable to detect undefined names	reviewbot	Jan. 20, 2016, 3:32 p.m.
'PIPELINE_CSS' imported but unused	reviewbot	Jan. 20, 2016, 3:32 p.m.
'PIPELINE_JS' imported but unused	reviewbot	Jan. 20, 2016, 3:32 p.m.
'django_reset' imported but unused	reviewbot	Jan. 21, 2016, 8:17 p.m.
'from settings_local import *' used; unable to detect undefined names	reviewbot	Jan. 21, 2016, 8:17 p.m.
'PIPELINE_CSS' imported but unused	reviewbot	Jan. 21, 2016, 8:17 p.m.
'PIPELINE_JS' imported but unused	reviewbot	Jan. 21, 2016, 8:17 p.m.
'django_reset' imported but unused	reviewbot	Jan. 25, 2016, 10:26 p.m.
'from settings_local import *' used; unable to detect undefined names	reviewbot	Jan. 25, 2016, 10:26 p.m.
'PIPELINE_CSS' imported but unused	reviewbot	Jan. 25, 2016, 10:26 p.m.
'PIPELINE_JS' imported but unused	reviewbot	Jan. 25, 2016, 10:26 p.m.
Should use url().	chipx86	Feb. 10, 2016, 7:08 p.m.
I might be wrong, but I think since we're just doing an include, you can put this up above where …	chipx86	Feb. 12, 2016, 10:44 p.m.
Alignment issue here.	chipx86	Feb. 12, 2016, 10:44 p.m.
No space after """. I'd also change this to "An OAuth2 client application registered by a user."	chipx86	Feb. 10, 2016, 7:15 p.m.
"URL" and "users".	chipx86	Feb. 10, 2016, 7:15 p.m.
Let's name this oauth2_apps_urlpatterns.	chipx86	Feb. 10, 2016, 7:16 p.m.
Let's put the view on its own line, for consistency.	chipx86	Feb. 10, 2016, 7:17 p.m.
Blank line between these.	chipx86	Feb. 10, 2016, 7:17 p.m.
This should probably go before the reviewboard imports. Ours should be last. Also, reviewboard.oauth2 should be in alphabetical order.	chipx86	Feb. 10, 2016, 8:21 p.m.
'django_reset' imported but unused	reviewbot	Jan. 26, 2016, 10:53 p.m.
'from settings_local import *' used; unable to detect undefined names	reviewbot	Jan. 26, 2016, 10:53 p.m.
'PIPELINE_CSS' imported but unused	reviewbot	Jan. 26, 2016, 10:53 p.m.
'PIPELINE_JS' imported but unused	reviewbot	Jan. 26, 2016, 10:53 p.m.
Can you document this?	chipx86	March 3, 2016, 9:51 a.m.
Should be listed in alphabetical order.	chipx86	Feb. 10, 2016, 8:22 p.m.
'django_reset' imported but unused	reviewbot	Feb. 12, 2016, 10:43 p.m.
'from settings_local import *' used; unable to detect undefined names	reviewbot	Feb. 12, 2016, 10:43 p.m.
'PIPELINE_CSS' imported but unused	reviewbot	Feb. 12, 2016, 10:43 p.m.
'PIPELINE_JS' imported but unused	reviewbot	Feb. 12, 2016, 10:43 p.m.
No space after """.	chipx86	March 3, 2016, 9:52 a.m.
Blank line between these.	chipx86	March 3, 2016, 9:52 a.m.
Is this covered in a future change? (I'll find out in a moment I'm sure). If so, we'll want to …	chipx86	March 4, 2016, 11:12 p.m.
'django_reset' imported but unused	reviewbot	Feb. 12, 2016, 10:46 p.m.
'from settings_local import *' used; unable to detect undefined names	reviewbot	Feb. 12, 2016, 10:46 p.m.
'PIPELINE_CSS' imported but unused	reviewbot	Feb. 12, 2016, 10:46 p.m.
'PIPELINE_JS' imported but unused	reviewbot	Feb. 12, 2016, 10:46 p.m.
'django_reset' imported but unused	reviewbot	March 4, 2016, 11:16 p.m.
'from settings_local import *' used; unable to detect undefined names	reviewbot	March 4, 2016, 11:16 p.m.

Tool: Pyflakes
Processed Files:
    reviewboard/oauth2/urls.py
    reviewboard/oauth2/models.py
    reviewboard/settings.py
    setup.py
    reviewboard/urls.py

Ignored Files:
    reviewboard/oauth2/__init__.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/oauth2/urls.py
    reviewboard/oauth2/models.py
    reviewboard/settings.py
    setup.py
    reviewboard/urls.py

Ignored Files:
    reviewboard/oauth2/__init__.py

reviewboard/oauth2/models.py (Diff revision 1)
Show all issues
```
Col: 5
 E265 block comment should start with '# '
```
reviewboard/oauth2/models.py (Diff revision 1)
Show all issues
```
Col: 9
 W292 no newline at end of file
```
reviewboard/oauth2/urls.py (Diff revision 1)
Show all issues
```
Col: 2
 W292 no newline at end of file
```
reviewboard/settings.py (Diff revision 1)
Show all issues
```
 'django_reset' imported but unused
```

reviewboard/settings.py (Diff revision 1)

Show all issues

 'from settings_local import *' used; unable to detect undefined names

reviewboard/settings.py (Diff revision 1)
Show all issues
```
 'PIPELINE_CSS' imported but unused
```
reviewboard/settings.py (Diff revision 1)
Show all issues
```
 'PIPELINE_JS' imported but unused
```
reviewboard/urls.py (Diff revision 1)
Show all issues
```
Col: 34
 W292 no newline at end of file
```

reviewboard/oauth2/models.py (Diff revision 1)

Show all issues

The first import should be:

from __future__ import unicode_literals

reviewboard/oauth2/models.py (Diff revision 1)

Show all issues

Docstrings should be of the form:

"""Single line summary.

Multi-line description.
"""

reviewboard/oauth2/models.py (Diff revision 1)
Show all issues
```
Blank line between these.
```
reviewboard/oauth2/models.py (Diff revision 1)
Show all issues
```
Unnecessary.
```

reviewboard/oauth2/urls.py (Diff revision 1)

Show all issues

We're only importing one set of views here. I think we should do:


from oauth2_provider import views

reviewboard/oauth2/urls.py (Diff revision 1)
Show all issues
```
"Review Board"

"OAuth"
Missing a period.
```
reviewboard/oauth2/urls.py (Diff revision 1)
Show all issues
```
Single quotes. Same below.
```
reviewboard/oauth2/urls.py (Diff revision 1)
Show all issues
```
"OAuth"
Also missing a period.
```
reviewboard/oauth2/urls.py (Diff revision 1)
Show all issues
```
Use patterns()
```
reviewboard/oauth2/urls.py (Diff revision 1)
Show all issues
```
No blank line here.
```
reviewboard/settings.py (Diff revision 1)
Show all issues
```
Missing a trailing comma.
```

reviewboard/urls.py (Diff revision 1)

Show all issues

Please keep these imports in alphabetical order.

reviewboard/urls.py (Diff revision 1)
Show all issues
```
Unnecessary comment.
```

reviewboard/urls.py (Diff revision 1)

Show all issues

This isn't the right place for this. account/ URLs should go in reviewboard/accounts/urls.py. There, you can leave off the account/ prefix (because all URLs there will have it automatically prepended because of how they are included).

lehoangm Jan. 20, 2016, 11:13 a.m.

I agree with the suggestion, so I will move them into reviewboard/accounts/urls.py but note that we will end up with multiple places that dealing with OAuth2

Change Summary:

Update review

Diff:

Revision 2 (+80 -1)

Show changes

	setup.py
	reviewboard/settings.py
	reviewboard/urls.py
	reviewboard/accounts/urls.py
	reviewboard/oauth2/__init__.py
	reviewboard/oauth2/models.py
	reviewboard/oauth2/urls.py

Tool: Pyflakes
Processed Files:
    reviewboard/settings.py
    reviewboard/oauth2/models.py
    reviewboard/accounts/urls.py
    reviewboard/oauth2/urls.py
    reviewboard/urls.py
    setup.py

Ignored Files:
    reviewboard/oauth2/__init__.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/settings.py
    reviewboard/oauth2/models.py
    reviewboard/accounts/urls.py
    reviewboard/oauth2/urls.py
    reviewboard/urls.py
    setup.py

Ignored Files:
    reviewboard/oauth2/__init__.py

reviewboard/settings.py (Diff revision 2)
Show all issues
```
 'django_reset' imported but unused
```

reviewboard/settings.py (Diff revision 2)

Show all issues

 'from settings_local import *' used; unable to detect undefined names

reviewboard/settings.py (Diff revision 2)
Show all issues
```
 'PIPELINE_CSS' imported but unused
```
reviewboard/settings.py (Diff revision 2)
Show all issues
```
 'PIPELINE_JS' imported but unused
```

Change Summary:

Update model name

Diff:

Revision 3 (+80 -1)

Show changes

	setup.py
	reviewboard/settings.py
	reviewboard/urls.py
	reviewboard/accounts/urls.py
	reviewboard/oauth2/__init__.py
	reviewboard/oauth2/models.py
	reviewboard/oauth2/urls.py

Tool: PEP8 Style Checker
Processed Files:
    reviewboard/settings.py
    reviewboard/oauth2/models.py
    reviewboard/accounts/urls.py
    reviewboard/oauth2/urls.py
    reviewboard/urls.py
    setup.py

Ignored Files:
    reviewboard/oauth2/__init__.py



Tool: Pyflakes
Processed Files:
    reviewboard/settings.py
    reviewboard/oauth2/models.py
    reviewboard/accounts/urls.py
    reviewboard/oauth2/urls.py
    reviewboard/urls.py
    setup.py

Ignored Files:
    reviewboard/oauth2/__init__.py

reviewboard/settings.py (Diff revision 3)
Show all issues
```
 'django_reset' imported but unused
```

reviewboard/settings.py (Diff revision 3)

Show all issues

 'from settings_local import *' used; unable to detect undefined names

reviewboard/settings.py (Diff revision 3)
Show all issues
```
 'PIPELINE_CSS' imported but unused
```
reviewboard/settings.py (Diff revision 3)
Show all issues
```
 'PIPELINE_JS' imported but unused
```

So I think you may be heading in the wrong direction here. You will probably want to expose setting OAuth2 configuration per-user via the API. You should ask Christian about this to double check.

LE

lehoangm Jan. 21, 2016, 8:16 p.m.
```
I will ask Christian to clarify more
```

lehoangm Jan. 22, 2016, 9:09 p.m.

Clarification:

oauth2_provider package provide me some oauth2 end points and some forms to manage list of applications.

Right now I am just put those end points and forms in a more proper place

Questions:

Do we plan to provide api support to manage oauth2 applications?

Should I rewrite the management api or try to wrap around and use as much as possible what oauth2_provider have for me ?

lehoangm Jan. 26, 2016, 10:52 p.m.

I discussed with Christian and it seems that there is no point adding API to manage OAuth2 client apps. Hence we are sticking to the form method for now

reviewboard/accounts/urls.py (Diff revision 3)
Show all issues
```
Alphabetize these.
```

reviewboard/accounts/urls.py (Diff revision 3)

Show all issues

This import doesn't need the as ... clause. When you do from x import y as y it is the same as doing from x import y.

reviewboard/accounts/urls.py (Diff revision 3)

Show all issues


url(r'^preferences/oauth2/',
    include(apps_urlpatterns, namespace='oauth2_provider'))

reviewboard/accounts/urls.py (Diff revision 3)

Show all issues

Why are we including this with a namespace?

lehoangm Jan. 21, 2016, 8:16 p.m.

oauth2_provider looking for that name space in the template system. Without this name space the current default template of oauth2_provider with throw errors.
Let's me add a TODO to remove this if needed since I am not very sure we still need this name space when we create our own template for OAuth2 forms.

LE

lehoangm Jan. 25, 2016, 10:27 p.m.
```
Removed URL
```

Change Summary:

Update review

Diff:

Revision 4 (+80 -1)

Show changes

	setup.py
	reviewboard/settings.py
	reviewboard/urls.py
	reviewboard/accounts/urls.py
	reviewboard/oauth2/__init__.py
	reviewboard/oauth2/models.py
	reviewboard/oauth2/urls.py

Tool: Pyflakes
Processed Files:
    reviewboard/settings.py
    reviewboard/oauth2/models.py
    reviewboard/accounts/urls.py
    reviewboard/oauth2/urls.py
    reviewboard/urls.py
    setup.py

Ignored Files:
    reviewboard/oauth2/__init__.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/settings.py
    reviewboard/oauth2/models.py
    reviewboard/accounts/urls.py
    reviewboard/oauth2/urls.py
    reviewboard/urls.py
    setup.py

Ignored Files:
    reviewboard/oauth2/__init__.py

reviewboard/settings.py (Diff revision 4)
Show all issues
```
 'django_reset' imported but unused
```

reviewboard/settings.py (Diff revision 4)

Show all issues

 'from settings_local import *' used; unable to detect undefined names

reviewboard/settings.py (Diff revision 4)
Show all issues
```
 'PIPELINE_CSS' imported but unused
```
reviewboard/settings.py (Diff revision 4)
Show all issues
```
 'PIPELINE_JS' imported but unused
```

Change Summary:

Move the OAuth2 client app management into RB API

Diff:

Revision 5 (+45)

Show changes

	setup.py
	reviewboard/settings.py
	reviewboard/urls.py
	reviewboard/oauth2/__init__.py
	reviewboard/oauth2/models.py
	reviewboard/oauth2/urls.py

Tool: PEP8 Style Checker
Processed Files:
    reviewboard/oauth2/urls.py
    reviewboard/oauth2/models.py
    reviewboard/settings.py
    setup.py
    reviewboard/urls.py

Ignored Files:
    reviewboard/oauth2/__init__.py



Tool: Pyflakes
Processed Files:
    reviewboard/oauth2/urls.py
    reviewboard/oauth2/models.py
    reviewboard/settings.py
    setup.py
    reviewboard/urls.py

Ignored Files:
    reviewboard/oauth2/__init__.py

reviewboard/settings.py (Diff revision 5)
Show all issues
```
 'django_reset' imported but unused
```

reviewboard/settings.py (Diff revision 5)

Show all issues

 'from settings_local import *' used; unable to detect undefined names

reviewboard/settings.py (Diff revision 5)
Show all issues
```
 'PIPELINE_CSS' imported but unused
```
reviewboard/settings.py (Diff revision 5)
Show all issues
```
 'PIPELINE_JS' imported but unused
```

Summary:

-	Add dependencies and urls to provide OAuth2 support for external services
+	[WIP] Add dependencies and urls to provide OAuth2 support for external services

Change Summary:

Remove WIP, update discussion

Summary:

-	[WIP] Add dependencies and urls to provide OAuth2 support for external services
+	Add dependencies and urls to provide OAuth2 support for external services

Diff:

Revision 6 (+80 -1)

Show changes

	setup.py
	reviewboard/settings.py
	reviewboard/urls.py
	reviewboard/accounts/urls.py
	reviewboard/oauth2/__init__.py
	reviewboard/oauth2/models.py
	reviewboard/oauth2/urls.py

Tool: Pyflakes
Processed Files:
    reviewboard/settings.py
    reviewboard/oauth2/models.py
    reviewboard/accounts/urls.py
    reviewboard/oauth2/urls.py
    reviewboard/urls.py
    setup.py

Ignored Files:
    reviewboard/oauth2/__init__.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/settings.py
    reviewboard/oauth2/models.py
    reviewboard/accounts/urls.py
    reviewboard/oauth2/urls.py
    reviewboard/urls.py
    setup.py

Ignored Files:
    reviewboard/oauth2/__init__.py

reviewboard/settings.py (Diff revision 6)
Show all issues
```
 'django_reset' imported but unused
```

reviewboard/settings.py (Diff revision 6)

Show all issues

 'from settings_local import *' used; unable to detect undefined names

reviewboard/settings.py (Diff revision 6)
Show all issues
```
 'PIPELINE_CSS' imported but unused
```
reviewboard/settings.py (Diff revision 6)
Show all issues
```
 'PIPELINE_JS' imported but unused
```

Change Summary:

Add reviewer + Add prefix to RC

Summary:

-	Add dependencies and urls to provide OAuth2 support for external services
+	[OAuth2Provider] Add dependencies and urls to provide OAuth2 support for external services

People:

+	brennie

reviewboard/accounts/urls.py (Diff revision 6)
Show all issues
```
Should use url().
```

reviewboard/accounts/urls.py (Diff revision 6)

Show all issues

I might be wrong, but I think since we're just doing an include, you can put this up above where we define the "preferences/" URL without having to worry about that view prefix argument to patterns causing problems.

lehoangm Feb. 10, 2016, 8:30 p.m.

I dont understand what you mean here. I just want oauth2 to be under preferences and under oauth2

lehoangm Feb. 12, 2016, 10:44 p.m.

I removed this to move all the management into the My Account page

reviewboard/accounts/urls.py (Diff revision 6)

Show all issues

Alignment issue here.

LE

lehoangm Feb. 10, 2016, 8:30 p.m.
```
I dont see any alignment issue here...
```

lehoangm Feb. 12, 2016, 10:44 p.m.

I removed this to move all the management into the My Account page

reviewboard/oauth2/models.py (Diff revision 6)

Show all issues

No space after """.

I'd also change this to "An OAuth2 client application registered by a user."

reviewboard/oauth2/urls.py (Diff revision 6)
Show all issues
```
"URL" and "users".
```
reviewboard/oauth2/urls.py (Diff revision 6)
Show all issues
```
Let's name this oauth2_apps_urlpatterns.
```

reviewboard/oauth2/urls.py (Diff revision 6)

Show all issues

Let's put the view on its own line, for consistency.

reviewboard/oauth2/urls.py (Diff revision 6)
Show all issues
```
Blank line between these.
```

reviewboard/settings.py (Diff revision 6)

Show all issues

This should probably go before the reviewboard imports. Ours should be last.

Also, reviewboard.oauth2 should be in alphabetical order.

reviewboard/settings.py (Diff revision 6)

Show all issues

Can you document this?

lehoangm Feb. 10, 2016, 8:30 p.m.

What document and where should I do it ?

chipx86 March 3, 2016, 12:35 a.m.

Just need to put comments above saying what this is for, and maybe explain that scopes are computing dynamically based on the API resources. Something like that. Just so people know what this structure is for when looking through the file.

setup.py (Diff revision 6)

Show all issues

Should be listed in alphabetical order.

Change Summary:

Update CR

Diff:

Revision 7 (+81 -1)

Show changes

	setup.py
	reviewboard/settings.py
	reviewboard/urls.py
	reviewboard/accounts/urls.py
	reviewboard/oauth2/__init__.py
	reviewboard/oauth2/models.py
	reviewboard/oauth2/urls.py

Tool: PEP8 Style Checker
Processed Files:
    reviewboard/settings.py
    reviewboard/oauth2/models.py
    reviewboard/accounts/urls.py
    reviewboard/oauth2/urls.py
    reviewboard/urls.py
    setup.py

Ignored Files:
    reviewboard/oauth2/__init__.py



Tool: Pyflakes
Processed Files:
    reviewboard/settings.py
    reviewboard/oauth2/models.py
    reviewboard/accounts/urls.py
    reviewboard/oauth2/urls.py
    reviewboard/urls.py
    setup.py

Ignored Files:
    reviewboard/oauth2/__init__.py

reviewboard/settings.py (Diff revision 7)
Show all issues
```
 'django_reset' imported but unused
```

reviewboard/settings.py (Diff revision 7)

Show all issues

 'from settings_local import *' used; unable to detect undefined names

reviewboard/settings.py (Diff revision 7)
Show all issues
```
 'PIPELINE_CSS' imported but unused
```
reviewboard/settings.py (Diff revision 7)
Show all issues
```
 'PIPELINE_JS' imported but unused
```

Change Summary:

Update oauth lib version and remove unessesary urls

Description:

		Added into list of installed apps package django-tool-kit which provides
		end-points and basic logic to manage client applications. All the end-points
		for OAuth2 authorization with external services is moved to /oauth2/.
~		All the tool to manage client applications is moved to
	~	All the tool to manage client applications is a part of preference menu
-		/account/preferences/oauth2/ since the OAuth setup is a part of preference menu

~		Added package django-tool-kit into setup.py so that it will get install with
~		ReviewBoard during setup time. The chosen version is the lasest version - 0.10
	~	Added package django-tool-kit into setup.py so that it will get install
	~	with ReviewBoard during setup time. Even though the lastest version is 0.10,
	+	the chosen version is 0.9 in order to be compatible with Django 1.6.

Diff:

Revision 8 (+44)

Show changes

	setup.py
	reviewboard/settings.py
	reviewboard/urls.py
	reviewboard/oauth2/__init__.py
	reviewboard/oauth2/models.py
	reviewboard/oauth2/urls.py

Tool: Pyflakes
Processed Files:
    reviewboard/oauth2/urls.py
    reviewboard/oauth2/models.py
    reviewboard/settings.py
    setup.py
    reviewboard/urls.py

Ignored Files:
    reviewboard/oauth2/__init__.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/oauth2/urls.py
    reviewboard/oauth2/models.py
    reviewboard/settings.py
    setup.py
    reviewboard/urls.py

Ignored Files:
    reviewboard/oauth2/__init__.py

reviewboard/settings.py (Diff revision 8)
Show all issues
```
 'django_reset' imported but unused
```

reviewboard/settings.py (Diff revision 8)

Show all issues

 'from settings_local import *' used; unable to detect undefined names

reviewboard/settings.py (Diff revision 8)
Show all issues
```
 'PIPELINE_CSS' imported but unused
```
reviewboard/settings.py (Diff revision 8)
Show all issues
```
 'PIPELINE_JS' imported but unused
```

Change Summary:

Update testing done

Testing Done:

~		Manually tested the urls are setup correctly
	~	Manually tested the urls are setup correctly
	+	Reran python setup.py develop

reviewboard/oauth2/models.py (Diff revision 8)
Show all issues
```
No space after """.
```
reviewboard/oauth2/models.py (Diff revision 8)
Show all issues
```
Blank line between these.
```

reviewboard/oauth2/models.py (Diff revision 8)

Show all issues

Is this covered in a future change? (I'll find out in a moment I'm sure). If so, we'll want to make sure one change has all the fields needed for a given model.

lehoangm March 3, 2016, 9:52 a.m.

Apparently after not having restriction on what localsite and what scope that an allication can be authenticated for, then we dont even need to change the original application class. Is it be better to remove this and use what from OAuth2 provider instead?

chipx86 March 4, 2016, 10:13 p.m.

Hmm... I think we probably want to have our own, because this may change in the future, and it'd be hard to update from another model. We should leave a comment (outside of the docstring) saying that we're subclassing this in order to have a model we can extend in the future.

Now that said, looking into this more, we actually can't subclass from Application. If you look at the code in oauth2_provider/models.py, you'll see that Application is a subclass (and therefore a working model in the database) of AbstractApplication. We're meant to be subclassing the latter. That's probably why you needed the app_label, which we also don't want (it'll result in conflicts between the two models).

lehoangm March 4, 2016, 11:14 p.m.

Even though, subclassing AbstractApplication creates unused table, after Slack discussion we agree that an extra table is not an issue so we should subclass the AbstractApplication

app_label can be removed, look at https://docs.djangoproject.com/en/dev/topics/auth/customizing/#auth-custom-user for more info

Change Summary:

Update CR

Diff:

Revision 9 (+48)

Show changes

	setup.py
	reviewboard/settings.py
	reviewboard/urls.py
	reviewboard/oauth2/__init__.py
	reviewboard/oauth2/models.py
	reviewboard/oauth2/urls.py

Tool: Pyflakes
Processed Files:
    reviewboard/oauth2/urls.py
    reviewboard/oauth2/models.py
    reviewboard/settings.py
    setup.py
    reviewboard/urls.py

Ignored Files:
    reviewboard/oauth2/__init__.py



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/oauth2/urls.py
    reviewboard/oauth2/models.py
    reviewboard/settings.py
    setup.py
    reviewboard/urls.py

Ignored Files:
    reviewboard/oauth2/__init__.py

reviewboard/settings.py (Diff revision 9)
Show all issues
```
 'django_reset' imported but unused
```

reviewboard/settings.py (Diff revision 9)

Show all issues

 'from settings_local import *' used; unable to detect undefined names

Change Summary:

Adding reminder

Description:

		Added into list of installed apps package django-tool-kit which provides
		end-points and basic logic to manage client applications. All the end-points
		for OAuth2 authorization with external services is moved to /oauth2/.
		All the tool to manage client applications is a part of preference menu

		Added package django-tool-kit into setup.py so that it will get install
		with ReviewBoard during setup time. Even though the lastest version is 0.10,
		the chosen version is 0.9 in order to be compatible with Django 1.6.
	+
	+	Note: please contact `lehoangminh@live.com` when you try to land the feature into master. I am more than happy to support the final integration

Review Board 6.0.1 alpha 0 (dev)

[OAuth2Provider] Add dependencies and urls to provide OAuth2 support for external services

Information

Reviewers

Change Summary:

Diff:

Change Summary:

Diff:

Change Summary:

Diff:

Change Summary:

Diff:

Summary:

Change Summary:

Summary:

Diff:

Change Summary:

Summary:

People:

Change Summary:

Diff:

Change Summary:

Description:

Diff:

Change Summary:

Testing Done:

Change Summary:

Diff:

Change Summary:

Description:

Status: Discarded