Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Auth grant jwt bearer support #2229

Closed
Xopek opened this issue Dec 8, 2020 · 10 comments
Closed

Auth grant jwt bearer support #2229

Xopek opened this issue Dec 8, 2020 · 10 comments
Labels
feat New feature or request.
Milestone

Comments

@Xopek
Copy link
Contributor

Xopek commented Dec 8, 2020

Hi! I want to implement authorization grant using JWT RFC.

In order to do this we first need to make changes in fosite, so it can support this grant. This is mostly done and soon i will open PR with it in fosite.

Second we need to store somewhere in hydra information how to check incoming authorization grant which is represented by jwt. In order to check jwt, we need to check it's signature (so we need public key), requested scopes, issuer claim and subject claim. Scopes and subject makes sense to attach to public key and public key to issuer. Because we can have issuer, that issued a lot of pairs for different assertions with different scopes and subjects.

I was suggested in Slack channel to open issuer and describe here how api in Hydra to store this new information can look like, so i will:

At first i would like to introduce new resource Issuer:

GET /issuer/{issuer_name} will return json like this:

{
  "name": "some-issuer",
  "created_at": "2019-08-24T14:15:22Z",
  "public_keys": [
    {
      "set": "set1",
      "kid": "04471f17-43bf-4953-aa5d-ae722ade6acc",
      "scopes": ["openid", "permissions"],
      "subject": "ro",
      "created_at": "2019-08-24T14:15:22Z",
      "expires_at": "2020-08-24T14:15:22Z"
    },
    {
      "set": "set1",
      "kid": "88871f17-43bf-4953-aa5d-ae722ade6acc",
      "scopes": ["openid"],
      "subject": "john@example.com",
      "created_at": "2019-08-24T14:15:22Z",
      "expires_at": "2020-08-24T14:15:22Z"
    },
    {
      "set": "set1",
      "kid": "388c1d17-43bf-4953-aa5d-ae722ade6acc",
      "created_at": "2019-08-24T14:15:22Z",
      "expires_at": "2020-08-24T14:15:22Z"
    }
  ]
}

To create Issuer we just make request like this

POST /issuers/{issuer_name} with contents similar to above, keys in POST will be represented with jwk.

Keys added in this manner can be managed using existing API /keys. Deleting key, will result in deleting it from issuer=>public_keys also.

To delete issuer completely, not just keys, we can make request:

DELETE /issuers/{issuer_name}

To add new key with scopes and subject to existing issuer:

PUT /issuers/{issuer_name}

@Xopek Xopek closed this as completed Dec 8, 2020
@Xopek Xopek reopened this Dec 8, 2020
@aeneasr
Copy link
Member

aeneasr commented Dec 8, 2020

High level, this looks good. I think however that we should allow multiple subjects. I also think that issuer might be a bit confusing wrt to the API naming. Maybe something like /grants/jwtbearer would be more straight forward? What do you think?

@aeneasr
Copy link
Member

aeneasr commented Dec 8, 2020

Also I would be ok with sending the JWK in the POST request!

@Xopek
Copy link
Contributor Author

Xopek commented Dec 9, 2020

High level, this looks good. I think however that we should allow multiple subjects. I also think that issuer might be a bit confusing wrt to the API naming. Maybe something like /grants/jwtbearer would be more straight forward? What do you think?

I think we should not allow multiple subjects.

Conceptually we are granting (that's why i love your variant with /grant/jwtbearer) some client ability to get access token and act on Behalf of a User. So ideally we have one concrete grant for one concrete subject (user).

If we draw parallels with normal authorization grant flow, there we also granting one-time permission for concrete client to represent us. If client wants to act on behalf of another user, it must again ask permission to do this. In normal flow, client explicitly asks for permission. In jwt bearer flow, "asking permission" is done in the way, that user shares private key with client.

So i think we must have unique key pair per each subject. That means we should not allow to have multiple subjects for one public key. The only exception i think can thinks of, is to make subject optional. If we won't provide subject, that means we are allowing to represent client any user, however we can achieve the same with anonymous value for subject. RFC also suggested this variant: For the authorization grant, the subject typically identifies an authorized accessor for which the access token is being requested (i.e., the resource owner or an authorized delegate), but in some cases, may be a pseudonymous identifier or other value denoting an anonymous user. That means we can subject as required parameter.

Considering API naming, i liked your variant. Endpoint /grants/jwtbearer makes much more sense. Also POST to /grants/jwtbearer will semantically mean: "i am granting permission to act on behalf of me(sub) for this issuer, for this scopes". Sounds very good!

@Xopek
Copy link
Contributor Author

Xopek commented Dec 9, 2020

However with the new name for endpoint /grant/jwtbearer i am struggling how request to get grant information will look like. Like this? GET /grant/jwtbearer/{grant_id} But then we will need some grant_id which we don't have. And from perspective of REST API /grant/jwtbearer/ looks like resource, but we need some kind of collection of resources... We can make something like this maybe then: /grant/jwtbearer/issuers/{issuer} and response will be the same as in my first message.

@Xopek
Copy link
Contributor Author

Xopek commented Dec 9, 2020

With grant_id request can look like this: GET /grants/jwtbearer/trusted-issuer-john

{
  "issuer": "trusted-issuer",
  "subject": "john",
  "created_at": "2019-08-24T14:15:22Z",
  "expires_at": "2020-08-24T14:15:22Z",
  "public_key": {
    "set": "set1",
    "kid": "04471f17-43bf-4953-aa5d-ae722ade6acc",
  },
  "scopes": [
    "openid",
    "permissions"
  ]
}

Looks good for me. What do you think? But still i don't know what to use as grant_id. Maybe just uuid. Then we can have two keys in storage, one uuid and one composite key of issuer+subject. Because during assertion checking we will have only subject and issuer to find appropriate grant.

@aeneasr
Copy link
Member

aeneasr commented Dec 10, 2020

If we draw parallels with normal authorization grant flow, there we also granting one-time permission for concrete client to represent us. If client wants to act on behalf of another user, it must again ask permission to do this. In normal flow, client explicitly asks for permission. In jwt bearer flow, "asking permission" is done in the way, that user shares private key with client.

So i think we must have unique key pair per each subject. That means we should not allow to have multiple subjects for one public key. The only exception i think can thinks of, is to make subject optional. If we won't provide subject, that means we are allowing to represent client any user, however we can achieve the same with anonymous value for subject. RFC also suggested this variant: For the authorization grant, the subject typically identifies an authorized accessor for which the access token is being requested (i.e., the resource owner or an authorized delegate), but in some cases, may be a pseudonymous identifier or other value denoting an anonymous user. That means we can subject as required parameter.

You convinced me - I like both approaches - explicit subject allow list plus "anonymous" option. Great!

Regarding the naming convention:

  • POST /grants/jwt-bearer/ creates a new (scope, public_key, subject) tuple and returns a UUID v4 as ID
  • GET /grants/jwt-bearer/<grant-id> retrieves the tuple
  • GET /grants/jwt-bearer retrieves all tuples with pagination
  • DELETE /grants/jwt-bearer/<grant-id> removes the tuple
  • PUT /grants/jwt-bearer/<grant-id> updates the tuple (ID can not be changed)

Payload would look like this (please notice that I changed scopes -> scope as we use singular in all other APIs):

{
  "id": "abc-123-uuid-v4",
  "subject": "john",
  "created_at": "2019-08-24T14:15:22Z",
  "expires_at": "2020-08-24T14:15:22Z",
  "public_key": {
    "set": "set1",
    "kid": "04471f17-43bf-4953-aa5d-ae722ade6acc",
  },
  "scope": [
    "openid",
    "permissions"
  ]
}

@Xopek
Copy link
Contributor Author

Xopek commented Dec 11, 2020

Thanks for your detailed answer, i will start implementing this)

@aeneasr
Copy link
Member

aeneasr commented Dec 11, 2020

Awesome! :)

@github-actions
Copy link

I am marking this issue as stale as it has not received any engagement from the community or maintainers in over half a year. That does not imply that the issue has no merit! If you feel strongly about this issue

  • open a PR referencing and resolving the issue;
  • leave a comment on it and discuss ideas how you could contribute towards resolving it;
  • open a new issue with updated details and a plan on resolving the issue.

We are cleaning up issues every now and then, primarily to keep the 4000+ issues in our backlog in check and to prevent maintainer burnout. Burnout in open source maintainership is a widespread and serious issue. It can lead to severe personal and health issues as well as enabling catastrophic attack vectors.

Thank you for your understanding and to anyone who participated in the issue! 🙏✌️

If you feel strongly about this issues and have ideas on resolving it, please comment. Otherwise it will be closed in 30 days!

@github-actions github-actions bot added the stale Feedback from one or more authors is required to proceed. label Sep 21, 2021
@aeneasr
Copy link
Member

aeneasr commented Sep 21, 2021

Marked as stale in error.

@aeneasr aeneasr removed the stale Feedback from one or more authors is required to proceed. label Sep 21, 2021
@aeneasr aeneasr added this to the next milestone Sep 21, 2021
@aeneasr aeneasr modified the milestones: next, unplanned Oct 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat New feature or request.
Projects
None yet
Development

No branches or pull requests

2 participants