Skip to content
This repository has been archived by the owner on Mar 11, 2021. It is now read-only.

WIP: Rework JWT with scopes in GOA design #2138

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kwk
Copy link
Collaborator

@kwk kwk commented Jun 28, 2018

Hey, I've looked at some swagger with @tinakurian and we found out about scopes in JWT and GOA. This PR exists to show how we might want to integrate scopes as presented by @alexeykazakov and @sbose78 into our endpoints.

The PR was inspired by this example usage of JWT: https://github.com/goadesign/examples/blob/master/security/design/jwt.go#L10

Notice that we were not using the newly created JWT global variable anywhere but just defined.

As you can see the scopes can be placed on each action of a resource which is close to what we want, no? What's missing in the generated swagger are information about the Authorization header that should be provided in requests. The required scopes for each action are appended to the description of each action.

I have no idea what effect the changes will have on our endpoints. Do the reject a request if a scope is missing now?

@alien-ike
Copy link

Ike Plugins (test-keeper)

Thank you @kwk for this contribution!

It appears that no tests have been added or updated in this PR.

Automated tests give us confidence in shipping reliable software. Please add some as part of this change.

If you are an admin or the reviewer of this PR and you are sure that no test is needed then you can use the command /ok-without-tests as a comment to make the status green.

For more information please head over to official documentation. You can find there how to configure the plugin.

@codecov-io
Copy link

codecov-io commented Jun 28, 2018

Codecov Report

Merging #2138 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2138   +/-   ##
=======================================
  Coverage   69.37%   69.37%           
=======================================
  Files         165      165           
  Lines       15277    15277           
=======================================
  Hits        10598    10598           
  Misses       3719     3719           
  Partials      960      960

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cd53cbe...460c7db. Read the comment docs.

@alexeykazakov
Copy link
Contributor

@kwk that's interesting yes. I didn't find what exactly is expected in the token by GOA JWT security middleware is some scopes are defined in design. But it looks like it's just top level scope claim.
It's not enough for out case. We will need to specify scopes for the resource ID (or set of resources). We use scopes per resource.
Another problem with this built-in middleware is that if there is no any information for the required resource in the token then WIT should not just reject access. It should try to obtain a new token from Auth.

We still could use GOA middlewares for that I think. But we will need to create our own custom one. We (auth team) also were thinking about creating common libs/tools for our go/Java/JS etc, clients/resource services which we could re-use instead of forcing each team to re-implement it from scratch.
We could do something for our goa repos/services too.

@kwk
Copy link
Collaborator Author

kwk commented Jul 3, 2018

But it looks like it's just top level scope claim.
It's not enough for out case. We will need to specify scopes for the resource ID (or set of resources). We use scopes per resource.

@alexeykazakov If I'm not mistaken you can put scopes on GOA resources and even actions (like I did) of a particular resource. Can you elaborate on how your resource definition differs from the GOA one?

@alexeykazakov
Copy link
Contributor

@kwk GOA resources != AuthZ resources.
If I understand correctly GOA JWT Middleware will check top level scope claim in the token when accessing some endpoint (resource/action):

“scopes”:[“view”,”contribute”]

But we need scopes for AuthZ resources, like spaces or org:

“resources”:[
{“id”:”12345”, “scopes”: [”view”]},
{“id”:”54321”, “scopes”: [”view”, “contribute”]}
]

See the difference?

In the first token there is global scopes but we need to define and check scopes per space as in the second token where “12345” and “54321” are space IDs.

@kwk
Copy link
Collaborator Author

kwk commented Jul 4, 2018

See the difference?

Yes I see that.

@kwk kwk added the idea An idea is a PR that shall not be deleted and captures a brain dump of some sort. label Aug 23, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🛀 cleanup help wanted idea An idea is a PR that shall not be deleted and captures a brain dump of some sort. 🐍 security work-in-progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants