One of the most convenient usage patterns for apps is to issue an Oauth2 password grant, which allows the user to directly exchange a username & password for an authorization token.

This is a first attempt at adding an endpoint that accepts password grants.

To use it, issue a request like:

POST /simple-oauth
Content-Type: application/json

{ "grant_type": "password", "username":"admin", "password: "xxx"}

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ef4 created an issue. See original summary.

e0ipso’s picture

This is a fantastic patch! Thank you Ed.

I have some nit picks:

  1. +++ b/simple_oauth.routing.yml
    @@ -117,3 +117,11 @@ access_token.refresh:
    +    _access: 'TRUE'
    

    I always forget, is this supposed to be TRUE or 'TRUE'?

  2. +++ b/src/Controller/AccessTokenIssue.php
    @@ -0,0 +1,115 @@
    +    $body = json_decode($request->getContent());
    

    Json::decode(…)

  3. +++ b/src/Controller/AccessTokenIssue.php
    @@ -0,0 +1,115 @@
    +      $this->response->setStatusCode(422);
    

    We should throw an HttpException instead. This is out of the scope of this issue, but … could you create a follow up for someone else to take on?

  4. +++ b/src/Controller/AccessTokenIssue.php
    @@ -0,0 +1,115 @@
    +    } else {
    

    DCS: else in the next line.

  5. +++ b/src/Controller/AccessTokenIssue.php
    @@ -0,0 +1,115 @@
    +  protected function normalize(AccessToken $token) {
    

    This needs a docblock.

e0ipso’s picture

I'm good with this feature request, but we need to figure out a way to make people understand that they can't allow 3rd parties to use this endpoint.

I do not think we can enforce this behavior from the server. We should add some big red signs explaining what this endpoint is used for.

Maybe this endpoint could respond with an error until the admin goes to the UI and "enables" it with a confirmation form.

Context:

The Password grant is used when the application exchanges the user’s username and password for an access token. This is exactly the thing OAuth was created to prevent in the first place, so you should never allow third-party apps to use this grant.

A common use for this grant type is to enable password logins for your service’s own apps. Users won’t be surprised to log in to the service’s website or native application using their username and password, but third-party apps should never be allowed to ask the user for their password.

ef4’s picture

Hi Mateu, I only just noticed your feedback (I guess email notification for issues was turned off, but now I figured out how to turn it on).

I came and looked because I saw you tweet about progress on a similar feature -- are you still interested in this one or has it been superceded? I am happy either way. If this patch is still valuable I will make the revisions you suggested.

e0ipso’s picture

Status: Needs review » Needs work

Yes. That tweet was about the version 8.x-2.x. However, I still think this issue has a lot of merit for 8.x-1.x so if you address the existing nit picks it would be fantastic.

On second read I don't think that we need anything extra regarding:

We should add some big red signs explaining what this endpoint is used for.

sylfo’s picture

Hi !

I gave this patch a try. However, I stumble on a HTTP error 500 when sending a POST on /simple-oauth. The apache logs reveals a dependency injection issue :

[Fri Nov 25 11:16:21.545541 2016] [proxy_fcgi:error] [pid 1291:tid 140214777378560] [client 192.168.88.1:49281] AH01071: Got error 'PHP message: Uncaught PHP Exception InvalidArgumentException: "Class "Drupal\\simple_oauth\\Controller\\AccessTokenIssue" does not exist." at /var/www/drupalvm/drupal/web/core/lib/Drupal/Core/DependencyInjection/ClassResolver.php line 24\n'

I'm totally newbie on Drupal/PHP and I can't see what is causing this issue. For info, a POST on /simple-oauth/refresh works fine.

Would you have any clue about this ?

Thanks !

sylfo’s picture

Just for information, I retried and realized I haven't reloaded Drupal cache (drush cache-reload). The route was not properly registered, leading to the 500 error. With the reload, it now works !

e0ipso’s picture

Status: Needs work » Fixed
FileSize
4.19 KB

Added some modifications before merging. Thanks @ef4!

  • e0ipso committed 15c9458 on 8.x-1.x authored by ef4
    feat(Authentication) Add password grant endpoint (#2821001 by ef4,...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.