taskcluster / taskcluster-rfcs

Taskcluster team planning
Mozilla Public License 2.0
11 stars 19 forks source link

RFC: Add scopes for anonymous calls #165

Closed ricky26 closed 3 years ago

ricky26 commented 4 years ago

An RFC to allow controlling access to all API endpoints in the TaskCluster services.

djmitche commented 3 years ago

For the case where anonymous access to a deployment is desired, I think there's still a compatibility issue here.

This RFC covers the situation where an API call is made without Hawk credentials, e.g., queue.task(someTaskId). But it doesn't cover the case where that such a call is made with Hawk credentials. I expect that's fairly uncommon, but there are probably cases where a credential-bearing client is handy and used to call an endpoint that doesn't require credentials. For example, a task might use the taskcluster-proxy interface to look up some index entries and later insert some index entries. Both calls would then be restricted with authorizedScopes to task.scopes. So, adopting the change would require identifying all the places this occurs and updating them to include the necessary scope (either specifically or just assume:anonymous).

I'm not sure how best to handle that. Thoughts?

ricky26 commented 3 years ago

@djmitche I'm not sure I entirely understand this edge-case. I don't know exactly how the proxy works but from what I understand, it uses the worker's authentication credentials and then narrows them by what the task is allowed to access. Credentials or not, assume:anonymous should be added to the authorized scopes for the call, so I wouldn't expect there to be any difference... In a private installation, without any other changes, tasks will require more scopes (in order to do some of the things previously not requiring scopes).

Have I missed something? 😅

petemoore commented 3 years ago

Have I missed something? 😅

@ricky26 I think @djmitche is referring to the two following issues:

petemoore commented 3 years ago

I'm not sure how best to handle that. Thoughts?

@djmitche On reflection, I think this could be solved with https://github.com/taskcluster/taskcluster-rfcs/pull/165#discussion_r488109238

ricky26 commented 3 years ago

Ah, right. I was expecting that we would always include the assume:anonymous scope for all calls (so that adding credentials could never restrict your access).

djmitche commented 3 years ago

Sounds like the issue is understood and some approaches for solving it are shaping up.

As a high-level design goal, I'd like to (a) limit special-casing and (b) put as much of that special-casing as possible into the auth service.

petemoore commented 3 years ago

The challenging question is whether clients that make an API call should have the anonymous scopes explicitly or implicitly, which is especially pertinent when scopes are limited explicitly to a set of authorized scopes, or a temporary client is used with an explicit list of scopes.

In some ways it feels clean for scopes always to be explicit: if you have a temporary client with an explicit list of authorized scopes, those are the scopes that define which API methods that client can call, and it shouldn't be able to call an API method that requires a scope that is not satisfied by that list.

However, on the other hand, any API call from any client should always have the anonymous scopes. So now you have a tricky situation that due to scope limiting techniques, the anonymous scopes may end up being filtered out. There is a knock-on effect too on scope expansion. Should all scope expansions always include the anonymous scopes, regardless of which scopes are being expanded? There may be a few touchpoints in the code that need careful thought.

I see the two main choices as:

a) Shovel anonymous scopes into all places where a list of scopes is generated (such as listing scopes for a given client or role, expanding scopes, ...) and then taking care to deal with cases where this list gets truncated in some way by some system, before being placed in a request that then requires these scopes, or

b) Make the scope satisfaction implicit, so that the anonymous scopes are considered by the authentication middleware to always be present. This has very few touchpoints, but does have the downside that it can be confusing to understand why an API call was allowed, when a client appears not to posses a required scope, for example.

Another option is a hybrid approach, with the aim of removing the implicit scope assignment at some point in the future:

c)

1) All clients are granted the assume:anonymous scope (explicit approach) 2) The auth middleware automatically includes assume:anonymous when there are AuthorizedScopes, and also for all temporary clients (that may also not have this scope) (implicit) 3) The auth middleware alerts when an implicit scope was required 4) We address the alerts raised in 3) by updating code that uses temp clients or authorized scopes to explicitly assign the required scopes 5) We remove the implicit assignment/alerting of scopes added in steps 2 and 3.

I feel like I can't see the wood for the trees now, so I'm not really sure what I prefer.

In any case, I think I'm pretty happy with any solution, so long as docs are updated (e.g. temp clients docs should state if they have anonymous scopes implicitly, authorized scopes docs should say if anonymous scopes are implicit or not, ...) and we have the anonymous scope caching to ensure we don't suffer a performance penalty for unauthenticated clients.

djmitche commented 3 years ago

That's a good point -- implementing this first without caching, then finding that we need to dial up the number of auth pods until we can implement caching in a followup, is an OK outcome. And it ensures that the design does not depend on caching.

djmitche commented 3 years ago

@escapewindow, we'd love your thoughts on this RFC :)

escapewindow commented 3 years ago

Interesting. I'm interested in the use case here: in your private cluster, you would remove the view scopes from assume:anonymous ?

I agree that allowing for private clusters may be important to gain more widespread usage of Taskcluster. Related proposals might include configuring which artifact prefix(es) are public and private, and where log artifacts go. I think if we're able to support private clusters without negatively affecting the FirefoxCI cluster, I'm good. 👍

ricky26 commented 3 years ago

@escapewindow yeah, that's exactly right - and then we'd assign them out probably to members of our GitHub teams (which seems like a pretty easy way of managing permissions). We don't have anything top secret but there's enough sensitive stuff in our logs and early game builds we don't want leaked!

My current goal is to make minimalistic proposals which make it possible to run private clusters. After we've got that, I'm looking forward to having longer-term discussions about exactly those kind of topics. 😄

djmitche commented 3 years ago

I agree that allowing for private clusters may be important to gain more widespread usage of Taskcluster. Related proposals might include configuring which artifact prefix(es) are public and private, and where log artifacts go. I think if we're able to support private clusters without negatively affecting the FirefoxCI cluster, I'm good. +1

Good points -- #159 might be related there, and we can mix in considerations about permissions and artifact names with the object service (#120, and probably yet another proposal for it)

ricky26 commented 3 years ago

I've reworked the changes from the PR conversation into the RFC text, assuming we'd go with (a) from @petemoore's summary. I've selected (a) as a default as it makes the most sense to me.

The two key issues which separate the 3 potential solutions in @petemoore's summary to my eye are:

Is there anything left I've missed? Does this solution make the most sense to people out of the alternatives?

djmitche commented 3 years ago

Let's take this to final-comment, if you're ready. That can also be a chance to test out posts to tools-taskcluster (ref https://github.com/taskcluster/taskcluster/issues/3539).

ccooper commented 3 years ago

We'll wrap up the Final Comment period on Monday, September 28.

ccooper commented 3 years ago

@ricky26 If you want to fix up the merge conflicts in README.md and rfcs/README.md, we can get this landed now.

ricky26 commented 3 years ago

Done! :)

ccooper commented 3 years ago

Thanks!