HTTP Basic Auth support for introspection (Fix issue #709)

[Abhishek8394]

Fixes#709

• Add a new mixin that allows authenticating with HTTP basic auth, credentials in body or access tokens
• Introduce and abstraction in views.generic to initialize the OauthLibMixin
• Change parent class of IntrospectTokenView from 'ScopedProtectedResourceView' to 'ClientProtectedScopedResourceView'

Checklist

• PR only contains one change (considered splitting up PR)
• unit-test added
• documentation updated
• CHANGELOG.md updated (only for user relevant changes)
• author name in AUTHORS

[coveralls]

Pull Request Test Coverage Report for Build 1344

• 33 of 34(97.06%) changed or added relevant lines in 5 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.03%) to 94.795%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
oauth2_provider/views/mixins.py	15	16	93.75%

Totals
Change from base Build 1341:	0.03%
Covered Lines:	1293
Relevant Lines:	1364

---

💛 - Coveralls

[Abhishek8394]

I can work on merging master in the branch if no one is already working on it

[auvipy]

could you check the latest failures?

[Abhishek8394]

Fixed the tests, issue was sending query params in a POST request. That is not allowed.
Also the travis test ran only on py34-django20? It should run on all versions till py37, correct?

[Abhishek8394]

Also maybe update the docs to tell people that sending query params in any request except GET is a bad request.

[Abhishek8394]

Fixed this. Can you merge this now?

…

***@***.**** commented on this pull request. ------------------------------ In oauth2_provider/views/mixins.py <#725 (comment)> : > + if request.method.upper() == "OPTIONS": + return super().dispatch(request, *args, **kwargs) + # Validate either with HTTP basic or client creds in request body. + # TODO: Restrict to POST. + valid = self.authenticate_client(request) + if not valid: + # Alternatively allow access tokens + # check if the request is valid and the protected resource may be accessed + valid, r = self.verify_request(request) + if valid: + request.resource_owner = r.user + return super().dispatch(request, *args, **kwargs) + else: + return HttpResponseForbidden() + else: + return super().dispatch(request, *args, **kwargs) no newline

[IvanAnishchuk]

I don't like the way tests were fixed. It might also be better to make a separate PR for test suite fixes. Accidentally, I prepared one already: #750

[Abhishek8394]

I got the test fixes from master. Is there anything more required to get this PR merged?

[vijithv]

Hi, any updates on merging this PR?

[vijithv]

Hey can we merge this? Tested the PR and it works fine. Just don't want to maintain a fork

what was asked for was done via #750

[n2ygk]

@Abhishek8394 sorry for the moving target but we have a new PR template that asks you to do a few things. Can you review this and check off as appropriate and push up a commit to do that. Specifically, I believe this is good new functionality so it would be great if you could make sure to update the CHANGELOG and documentation. Also credit where credit is due: please add your name to AUTHORS.

Also, it appears the flake8 test (that somebody had incorrectly commented out of tox.ini and is now restored) is now failing, so you'll need to do some minor fixes.

Checklist

• PR only contains one change (considered splitting up PR)
• unit-test added
• documentation updated
• CHANGELOG.md updated (only for user relevant changes)
• author name in AUTHORS

[Abhishek8394]

I'll get a fix out in a day or two. In the meanwhile do verify the checklist at the top of the page.

[n2ygk]

@Abhishek8394 After pushing you to improve documentation, I'm embarrassed to admit that when I added RESOURCE_SERVER_INTROSPECTION_CREDENTIALS to settings, I failed to document it properly in #557! a093565 fixes that.

But have you documented somewhere how to configure use of HTTP Basic Auth for the introspect endpoint? Can you please add that?

[Abhishek8394]

There is not setting or configuration required, it just works and is backwards compatible so existing users won't fail. Detail explanation below:

So previously introspection only used to work with access tokens. Now it checks for client authentication first, if that fails then it falls back to checking the access token. Client authentication works as follows:

• Check for HTTP Basic Auth
• If prev fails, check for client_id and client_secret in request body (get/post params).
• If prev fails, fail client auth.

I think it should be made clear that client_id and client_secret are credentials for each app, not something server-wide. So a user uses the client_id and secret corresponding to the app which is requesting the introspection.

[Abhishek8394]

Also documentation at #setup-the-resource-server seems to document about using HTTP basic Auth for introspection. Let me know if I'm missing something on this.