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

Scope expression template registration #136

Merged
merged 4 commits into from
Jan 31, 2019
Merged

Conversation

imbstack
Copy link
Contributor

@imbstack imbstack commented Dec 9, 2018

This proposes a way of centralizing knowledge of what scopes allow
across an entire cluster.

It is sort of hand-wavy at the moment but I think it is possible. I've
written a small tool that can take a scope expression template with
scopes and allow us to know if it might be satisfied and which
parameters would be allowed.

@imbstack imbstack force-pushed the scope-expression-reg branch from 1485512 to dd860b1 Compare December 9, 2018 02:07
@imbstack
Copy link
Contributor Author

imbstack commented Dec 9, 2018

Just roping in a couple people I know will care at first. Will expand to rest of team after we mull this over a bit.

rfcs/0136-scope-expression-registration.md Outdated Show resolved Hide resolved
rfcs/0136-scope-expression-registration.md Show resolved Hide resolved
rfcs/0136-scope-expression-registration.md Show resolved Hide resolved
rfcs/0136-scope-expression-registration.md Outdated Show resolved Hide resolved
rfcs/0136-scope-expression-registration.md Show resolved Hide resolved
require a scope to register actions in that namespace. For instance, only taskcluster-github will be allowed
to register the `github.createStatus` action. This action will have associated metadata including the scope
expression template that guards it. This registration must be versioned and have an expiration date.
1. Auth service will grow an `authorize()` endpoint that will take as input an action, a clientId, and a set of
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this will be a little more complex than this -- we need to support both the equivalent of authenticateHawk (where the caller sends along the Hawk header and the HTTP details needed to verify it) and something to support workers (not sure how this would work).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to think about this more. Maybe we should have a vidyo about it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specifically this means that we still want a way to ask about a dynamic set of scopes, and not just for a client Id.

rfcs/0136-scope-expression-registration.md Show resolved Hide resolved
rfcs/0136-scope-expression-registration.md Outdated Show resolved Hide resolved

Before the service is started, when the cluster admin creates a client for this service, they will
ensure that it has the scope `auth:register:dishwasher`. When the service is started, it will register
this action and the associated terms with the auth service.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we talked about a second set of scopes that allow access to specific scopes here (that's going to be confusing..). So something like auth:require-scope:dishwasher:*. I think in most cases these will be the same prefix (well, dishwasher.* and dishwasher:* here), but it might be more illustrative if they are different in the example. Maybe the scope is dishes:wash:<detergent> instead?

In real life, I can see a case where multiple worker implementations use the same scope space for caches, for example, so worker manager would have auth:register-scope:worker-cache:*.


This does require auth to be running for any other service to start. Maybe we can be smart by retrying
registration until auth is running or just always checking to see if auth has the most recent version
of the definition every time we call `authorize()`?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's OK for a service to not really be "up" until auth is started -- after all, it's unlikely to work correctly without auth anyway. Running in Kubernetes means that the existing strategy of exiting the process when startup fails is a good fix, since k8s will automatically restart the process -- so when auth finally comes up, the dependent service will too (Heroku isn't so nice..)

Copy link
Contributor

@jhford jhford left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I definitely have feelings on this, but they're in my gut more than head. I think the root of these feelings is that scopes and auth are getting really hard to understand from end to end. Also, the registration model makes me a little nervous.

At the same time, I'm not sure that I have any useful feedback on this RFC and don't want to block it.

rfcs/0136-scope-expression-registration.md Show resolved Hide resolved
1. We will add a new field to our service api definitions at all of the cluster, service, and endpoint levels
that allows us to define terms at their most appropriate level. No overriding will be permitted and each
term must be defined with both human language documentation and a regex defining valid values for parameters
submitted for these terms.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How will we guarantee that the human readable language matches the regex? Defining security things twice, especially in different forms feels less than ideal. What if we changed the format to make it easy to generate a human readable string based on the non-regex declaration?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the regex and the description give different information: acceptable values, vs. "meaning".

For example, the term "hookId" might be defined as "A user-supplied identifier for a single hook, unique within the scope of an associated hookGroupId". The regex would be /^([a-zA-Z0-9-_]*)$/.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the regex and the description give different information: acceptable values, vs. "meaning".

Yes, I think it's important to document the intention behind the regexp because that will usually make it easier to do a proper review and validation.

@owlishDeveloper owlishDeveloper self-requested a review January 15, 2019 18:08
rfcs/0136-scope-expression-registration.md Outdated Show resolved Hide resolved
@imbstack imbstack force-pushed the scope-expression-reg branch from ce3606c to 6ce42a9 Compare January 17, 2019 18:04
Copy link
Contributor

@owlishDeveloper owlishDeveloper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I want to have taskcluster-dishwasher in owlish-kitchen 😭 )

@imbstack imbstack merged commit 5b9fcd9 into master Jan 31, 2019
@imbstack imbstack deleted the scope-expression-reg branch January 31, 2019 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants