Jwt Bearer is not sure whether "sub" contains the username or the user email.

[bojanz]

JWTBearerTest says "@param $sub The subject we are acting on behalf of. This could be the email address of the user in the system."
The email address is what examples elsewhere use for the subject field as well.

However, the JwtBearer grant type has:

    public function getUserId()
    {
        return $this->jwt['sub'];
    }

This treats the subject as the username (since user id is always the username in the library.)
So, either the library changes its tests and code docs to indicate that "sub" contains a username, or it introduces a new storage method for loading the user by mail, and uses that in the JwtBearer grant type.

Semi-related question: Why does getClientKey($client_id, $subject) accept the $subject? Is it likely that a client would have a public key per user? And is there a source (spec?) for that?

[bshaffer]

@bojanz excellent questions.

1. As far as the user_id question goes, the entire concept of a user identifier is not defined in this library, and is defined by the server implementing the library. Yes, we have the UserStorage, but that is the most rhudimentary of all the storages implemented in the library, and my hope has always been that someone will develop adaptors for Drupal, Wordpress, FOSUserBundle, etc. So in conclusion, if we update the documentation to make this clear, will that suffice?
2. I have been wondering the same thing. In Adobe's implementation, we simply ignore the $subject parameter. It does not make sense (to me) to have a public key per client per user. If this implementation is desired, then this can be done by the application desiring it. I have to double check the spec to be certain, however. The author of this grant type is @dsquier. Perhaps he can shed some light on it.

[bojanz]

1. You still need to be consistent in your examples. All of the storages you ship (pdo, mongo, etc) assume user_id is the username. And then BAM, in JWT user_id is the mail. Drupal follows the user_id -> username assumption and JWT simply cannot work because of this.
2. Yeah, I'm ignoring the subject as well in the initial patch.

[F21]

Hey guys, I implemented JWT bearers early last year, so things could have changed.

The spec I used then was: http://tools.ietf.org/html/draft-ietf-oauth-jwt-bearer-01

Where the spec was not clear, I followed google's implementation: https://developers.google.com/accounts/docs/OAuth2ServiceAccount

1. The userId is whatever you use in your application. Some application use a username (wordpress) other uses email addresses (facebook). It's entirely up to you and both are equally valid.
2. In google's implementation, each subject was given a separate key. That's what I have done in the implementation. Personally, I feel that this also affords some flexibility. If you feel that you need to have keys for each subject, the ability is there, but if you don't need that functionality you can ignore it. For me, having separate keys for each client to subject pair makes sense, because we are treating the whole JWT as a unique unit.

[luxifer]

Why can't we have in the current implementation a public key per client only and not per client per user? Because if you want to use the JWT as an authorization grant you should have a subject. And for now the getClientKey is done by passing the issuer AND the subject. For me it's really painful to have to generate a private/public for each user/client

[F21]

@luxifer: I agree. Clients should be able to have 1 key and impersonate other clients via the subject parameter.

The default storage implementation for the getClientKey() method in the storages interface unfortunately searches for a unique key for each client/subject pair when I implemented the JWT Bearer token a while back.

However, all is not lost! If you are using a custom written storage, simply use getClientKey() to check to see if the client can impersonate the subject and then return the client's key. If you are using the provided storages, a quick workaround would be to extend the storage class and override getClientKey().

I am currently a bit busy, but it would great if anyone can make this change to getClientKey(). This will also introduce some changes to the database schema resulting in breaking BC, but migration should not be too painful.

[luxifer]

That's what i've done. I just override the getClientKey in my custom
storage to only check the client_id

[F21]

You shouldn't just be checking the client_id though, you should check to see if client_id can impersonate the subject.

In any case, do you think you can submit a PR?

[luxifer]

In my case client_id is highly trusted so i don't have to check if it can impersonate the user. My purpose is to only use the JWT grant type for highly trusted client and Authorization Code or Implicit for others

[ghost]

Why is called "client_id" at first place? It's actually an issuer(URL) of the token that is pushed in, therefore it's issuer's public key and has nothing to do with client. Client is stored under "aud" field inside JWT Token.

http://openid.net/specs/openid-connect-core-1_0.html#IDToken

Referring to this line in JWTBearer(176):

if (!$key = $this->storage->getClientKey($jwt['iss'], $jwt['sub'])) {

[F21]

@valheranze It's called client_id() to maintain consistency with other grant types in the library. This is the spec we are using: http://tools.ietf.org/html/draft-ietf-oauth-jwt-bearer-09

[ghost]

@F21 ahhh true, see now the difference between oauth draft and openid connect draft.

[ghost]

@F21 still, there is inconsistency in library, looking inside IdToken(60-62) when creating id_token:

'iss'        => $this->config['issuer'],
'sub'        => $user_id,
'aud'        => $client_id,

'aud' should then be issuer(https://example.info/uri) and 'iss' identity of the client(client_id).

[F21]

@valheranze I am not familiar with the open id spec for JWT tokens, but the spec you linked to shows that the implementation is correct:

iss REQUIRED. Issuer Identifier for the Issuer of the response. The iss value is a case sensitive URL using the https scheme that contains scheme, host, and optionally, port number and path components and no query or fragment components. 

sub REQUIRED. Subject Identifier. A locally unique and never reassigned identifier within the Issuer for the End-User, which is intended to be consumed by the Client, e.g., 24400320 or AItOawmwtWwcT0k51BayewNvutrJUqsvl6qs7A4. It MUST NOT exceed 255 ASCII characters in length. The sub value is a case sensitive string.

aud REQUIRED. Audience(s) that this ID Token is intended for. It MUST contain the OAuth 2.0 client_id of the Relying Party as an audience value. It MAY also contain identifiers for other audiences. In the general case, the aud value is an array of case sensitive strings. In the common special case when there is one audience, the aud value MAY be a single case sensitive string. 

The JWT bearer token and open id implementation are 2 separate things. I do agree its confusing that open id's JWT and JWT bearer uses different things for the JWT's components.

[ghost]

@F21 hmm if JWT Bearer is not the correct GrantType to be used for openid connect implementation, which one is then?

[F21]

@valheranze I have not used the Open ID implementation, but I assume that you will need to use the AuthorizationCode GrantType located in https://github.com/bshaffer/oauth2-server-php/blob/develop/src/OAuth2/OpenID/GrantType/AuthorizationCode.php

cc @bshaffer Can we have a cookbook for open id in the documentation?